feat(plugins): add development plugin to monorepo #5490
Conversation
Merge @eggjs/development plugin from its separate repository into the monorepo under plugins/development directory. This establishes the plugins directory structure for all Egg framework plugins. - Create plugins/ directory for all framework plugins - Migrate @eggjs/development from https://github.com/eggjs/development - Update to use workspace dependencies and catalog entries - Configure tsdown for building (replacing tshy) - Update pnpm-workspace.yaml to include plugins/* - Update egg package to use development as workspace dependency - Document plugin structure in CLAUDE.md BREAKING CHANGE: @eggjs/development is now part of the monorepo 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a new top-level plugins/development Egg plugin (runtime code, config, types, loader-trace middleware), test suites and fixtures, packaging/build/test configs for the plugin, workspace and TS/Vitest updates, and switches Changes
Sequence Diagram(s)sequenceDiagram
participant FS as File System
participant Watch as watcher
participant Agent as AgentBoot (plugin)
participant Master as Master Process
participant Worker as App Worker
Note over Agent: Development-time file watcher flow
FS->>Watch: file change in watchDirs
Watch->>Agent: change event (path)
Agent->>Agent: apply ignoreDirs / reloadPattern filters
alt passes filters
Agent->>Agent: debounce 200ms
Agent-->>Master: process.send('reload-worker', { path })
Master->>Worker: reload workers
else ignored
Agent-->>Agent: no-op
end
Note over Agent,Worker: if process_mode == 'single' -> skip reload
sequenceDiagram
participant Client as Browser
participant App as Egg App
participant MW as egg_loader_trace middleware
participant FS as run dir
Client->>App: GET /__loader_trace__
App->>MW: handle request
MW->>FS: read timing files (agent/app timing JSON)
FS-->>MW: timing items[]
MW->>MW: inject JSON into HTML template
MW-->>Client: 200 HTML (chart + data)
sequenceDiagram
participant App as Application
participant Boot as AppBoot (plugin)
App->>Boot: construct(app)
Boot->>App: configWillLoad() pushes 'eggLoaderTrace' middleware
alt config.development.fastReady === true
Boot->>App: nextTick(() => app.ready(true))
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying egg with
|
| Latest commit: |
fe67faf
|
| Status: | 🚫 Build failed. |
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the @eggjs/development plugin from its separate repository into the monorepo under a new plugins/ directory structure, establishing a centralized location for all Egg framework plugins.
- Adds plugins/ directory to pnpm-workspace.yaml for monorepo workspace management
- Migrates complete @eggjs/development source code, tests, and configuration from external repository
- Updates build system from tshy to tsdown and converts dependencies to workspace references
Reviewed Changes
Copilot reviewed 55 out of 67 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pnpm-workspace.yaml | Adds plugins/* to workspace, removes catalog entry, adds new dependencies |
| plugins/development/* | Complete migration of development plugin source code, tests, and configuration |
| packages/egg/package.json | Updates @eggjs/development dependency to use workspace reference |
| CLAUDE.md | Documents new plugins directory structure and guidelines |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @fengmk2, 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 significantly refactors the project's structure by integrating the @eggjs/development plugin directly into the monorepo. This move establishes a dedicated plugins/ directory for all framework plugins, streamlining dependency management through workspace protocols and standardizing the build process for these internal components. The change aims to improve consistency, simplify development workflows, and centralize the codebase for better maintainability.
Highlights
- Monorepo Structure Enhancement: A new
plugins/directory has been introduced at the root level to house all Egg framework plugins, centralizing their management within the monorepo. - Development Plugin Migration: The
@eggjs/developmentplugin has been merged from its standalone repository into the monorepo underplugins/development, establishing the new plugin directory structure. - Dependency Management Update: The
packages/egg/package.jsonnow references@eggjs/developmentusingworkspace:*, aligning with monorepo best practices for internal dependencies. Thepnpm-workspace.yamlhas also been updated to include the newplugins/*path. - Build System Configuration: The development plugin's build process has been configured to use
tsdown, replacingtshy, and its TypeScript configuration has been integrated into the roottsconfig.json. - Documentation Updates: The
CLAUDE.mdfile has been updated to reflect and document the new plugin directory structure and guidelines for adding new packages.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
Deploying egg-v3 with
|
| Latest commit: |
fe67faf
|
| Status: | 🚫 Build failed. |
There was a problem hiding this comment.
Code Review
This pull request successfully migrates the @eggjs/development plugin into the monorepo, including its source code, tests, and documentation. The configuration changes across the monorepo, such as in pnpm-workspace.yaml and tsconfig.json, are correct and align with the goal of integrating the plugin. My review includes several suggestions to improve code quality and correctness: fixing a syntax error in a documentation example, resolving an inconsistency in the required Node.js version, replacing a deprecated API with a more robust alternative, improving type safety in a middleware, fixing a potential bug in a utility function, and cleaning up the new test files by removing debugging statements and addressing tests skipped on CI.
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/egg/package.json (1)
58-106: Critical: packages/egg is ESM-only — breaks CommonJS consumerspackages/egg/package.json has "type":"module" with main/module -> ./dist/index.js and publishConfig.exports -> ./dist/*.js; packages/egg/dist is absent and packages/egg/tsdown.config.ts is configured unbundle (ESM) — no CJS artifacts found. This will break require('egg'). Action: either (A) produce CJS artifacts (e.g., dist/index.cjs) and add conditional exports + main -> ./dist/index.cjs / module -> ./dist/index.js / exports.{".":{ "import": "./dist/index.js", "require": "./dist/index.cjs", "types":"./dist/index.d.ts" }}; or (B) declare Egg 4 ESM-only and document migration. Location: packages/egg/package.json.
tsconfig.json (1)
1-8: Enable strict at the root if not already provided by @eggjs/tsconfig."compilerOptions": { "target": "ES2022", "composite": true, "incremental": true, - "noEmit": true + "noEmit": true, + "strict": true },CLAUDE.md (1)
215-221: Align plugin tests with repo policy (Vitest) or document exception.plugins/development/package.json uses "egg-bin test" and tests in plugins/development/test/ use global describe/it without importing from 'vitest' — convert these tests to Vitest (add vitest devDependency and import { describe, it } from 'vitest' or enable vitest globals) or document a justified exception.
🧹 Nitpick comments (43)
plugins/development/test/fixtures/development/package.json (1)
1-3: Mark fixture as private and add a version to avoid accidental publish/tooling hiccups.
Minimal, safe metadata for fixtures.Apply this diff:
{ - "name": "development" + "name": "development", + "version": "0.0.0", + "private": true }plugins/development/test/fixtures/custom/package.json (1)
1-3: Mark fixture package as private to avoid accidental publishAdd
"private": trueto prevent accidental publication of test fixtures.{ - "name": "custom" + "name": "custom", + "private": true }plugins/development/test/fixtures/override/package.json (1)
1-3: Set fixture to privatePrevents accidental publish and keeps tooling from treating it as a real package.
{ - "name": "override" + "name": "override", + "private": true }plugins/development/test/fixtures/timing/package.json (1)
1-3: Set fixture to privateSame rationale as other fixtures.
{ - "name": "fast-ready" + "name": "fast-ready", + "private": true }plugins/development/test/fixtures/delay-ready/package.json (1)
1-3: Set fixture to privatePrevent accidental publish.
{ - "name": "delay-ready" + "name": "delay-ready", + "private": true }plugins/development/test/fixtures/override-ignore/package.json (1)
1-3: Set fixture to privateAvoid accidental publication.
{ - "name": "override-ignore" + "name": "override-ignore", + "private": true }plugins/development/test/fixtures/absolute/package.json (1)
1-3: Set fixture to privateKeep fixtures out of any publish flow.
{ - "name": "absolute" + "name": "absolute", + "private": true }plugins/development/test/fixtures/development-ts/tsconfig.json (1)
1-10: TS config looks good; optional tighten include/excludeStrict mode + NodeNext is appropriate. Optionally bound the file set to speed editor tooling.
{ "extends": "@eggjs/tsconfig", "compilerOptions": { "strict": true, "noImplicitAny": true, "target": "ES2022", "module": "NodeNext", "moduleResolution": "NodeNext" - } + }, + "include": ["./**/*.ts"], + "exclude": ["node_modules"] }plugins/development/test/fixtures/development-ts/package.json (1)
1-4: Mark fixture package as private to avoid accidental publish.Apply:
{ "name": "development-ts", - "type": "module" + "type": "module", + "private": true }plugins/development/test/fixtures/development/config/config.default.js (1)
1-1: Align with other fixtures: add 'use strict'.+'use strict'; exports.keys = 'foo,bar';plugins/development/test/fixtures/not-reload/package.json (1)
1-3: Mark fixture package as private.{ "name": "not-reload" + ,"private": true }plugins/development/test/fixtures/custom/config/config.default.js (1)
3-5: Custom reload pattern—OK; consider dotfiles behavior.If you need dotfiles reloading, add
'.*'explicitly as some globs ignore them by default.exports.development = { - reloadPattern: ['**', '!**/*.ts'], + reloadPattern: ['**', '.*', '!**/*.ts'], };plugins/development/tsdown.config.ts (1)
4-4: Consider explicit entries to control surface (optional).If only a few entry points are meant to be public (e.g.,
app.ts,agent.ts, middleware), prefer explicit entries to avoid accidental publishes.- entry: 'src/**/*.ts', + entry: [ + 'src/app.ts', + 'src/agent.ts', + 'src/app/middleware/egg_loader_trace.ts', + 'src/types.ts', + ],plugins/development/test/fixtures/development/app/router.js (1)
1-9: Prefer ctx over this in async handlers for consistency.The TS fixture uses
ctx; aligning avoids confusion aroundthisbinding in async functions.-module.exports = app => { - app.get('/foo.js', async function () { - this.body = 'foo.js'; - }); - - app.get('/foo', async function () { - this.body = 'foo'; - }); -}; +module.exports = app => { + app.get('/foo.js', async ctx => { + ctx.body = 'foo.js'; + }); + + app.get('/foo', async ctx => { + ctx.body = 'foo'; + }); +};plugins/development/test/fixtures/timing/app/router.js (1)
1-4: Prefer router API over app.get for clarity/typingUse app.router.get (Egg’s canonical router) to avoid ambiguity.
-module.exports = app => { - app.get('/checkFile', async ctx => { +module.exports = app => { + app.router.get('/checkFile', async ctx => { ctx.body = ctx.app.checkFile(); }); };Is app.get a supported alias in your current Egg version? If yes, feel free to keep it; otherwise adopt the diff above.
plugins/development/test/utils.ts (1)
4-5: Minor: use a single, consistent derivation for __dirnameThis is fine; just ensure consumers don’t re-export a conflicting __dirname elsewhere.
plugins/development/test/not-reload.test.ts (2)
4-4: Avoid shadowing global escape in importsAlias the import (or adopt the utils.ts rename).
-import { escape, getFilepath, DELAY } from './utils.js'; +import { escapeRegExp as escape, getFilepath, DELAY } from './utils.js';And keep usage as escape(...) or rename the call site to escapeRegExp(...).
11-16: Potential inspector port conflicts in CIUsing --inspect binds to 9229 by default; parallel tests may collide.
Consider disabling inspector for this test or using a random port (e.g., --inspect=0 if supported).
Please confirm whether your CI runs tests in parallel; if so, prefer a randomized inspector port or remove inspector for this case.
plugins/development/src/utils.ts (1)
17-19: Optional: be explicit about basename in isTimingFileIf callers pass full paths, the regex won’t match. Either document that file must be a basename or normalize here.
-export function isTimingFile(file: string) { - return /^(agent|application)_timing/.test(file); -} +export function isTimingFile(file: string) { + const name = path.basename(file); + return /^(agent|application)_timing/.test(name); +}plugins/development/test/absolute.test.ts (1)
5-5: Stabilize timing and drop noisy console.log
- Use shared DELAY constant to reduce flakiness.
- Remove debug console.log to keep CI logs clean.
-import { getFilepath } from './utils.js'; +import { getFilepath, DELAY } from './utils.js'; @@ - console.log(`write file to ${filepath}`); + // write file to trigger reload @@ - await scheduler.wait(1000); + await scheduler.wait(DELAY); @@ - await scheduler.wait(5000); + await scheduler.wait(DELAY);Also applies to: 32-36
plugins/development/test/custom.test.ts (1)
30-32: Add a short post-unlink wait to avoid race with log emissionWithout a tiny wait after unlink, assertions can race the watcher logging.
await fs.unlink(filepath); - app.expect('stdout', /a\.js/); + await scheduler.wait(500); + app.expect('stdout', /a\.js/); @@ await fs.unlink(filepath); - app.notExpect('stdout', /b\.ts/); + await scheduler.wait(500); + app.notExpect('stdout', /b\.ts/);Also applies to: 37-38
plugins/development/test/override.test.ts (1)
4-4: Rename imported escape to avoid shadowing the global escape()Biome flags this. Alias the util to a clearer name and update usages.
-import { escape, getFilepath } from './utils.js'; +import { escape as escapeRe, getFilepath } from './utils.js'; @@ - app.expect( + app.expect( 'stdout', - new RegExp(escape(`reload worker because ${filepath}`)) + new RegExp(escapeRe(`reload worker because ${filepath}`)) ); @@ - app.notExpect( + app.notExpect( 'stdout', - new RegExp(escape(`reload worker because ${filepath} change`)) + new RegExp(escapeRe(`reload worker because ${filepath} change`)) );Also applies to: 58-61, 71-74
plugins/development/test/development.test.ts (2)
5-5: Fix escape() shadowing by aliasing importAvoid shadowing the global and keep naming clear.
-import { escape, getFilepath, DELAY } from './utils.js'; +import { escape as escapeRe, getFilepath, DELAY } from './utils.js'; @@ - app.expect( + app.expect( 'stdout', - new RegExp(escape(`reload worker because ${filepath}`)) + new RegExp(escapeRe(`reload worker because ${filepath}`)) ); @@ - app.notExpect( + app.notExpect( 'stdout', - new RegExp(escape(`reload worker because ${filepath}`)) + new RegExp(escapeRe(`reload worker because ${filepath}`)) );Also applies to: 27-31, 39-42
45-45: Avoid dynamic RegExp; align test name with expectation
- Use a safe string count to dodge ReDoS lint.
- The test expects 4 logs; rename to reflect reality.
-it('should reload once when 2 file change', async () => { +it('should log 4 reload messages when 2 files change', async () => { @@ -function count(str: string, match: string) { - const m = str.match(new RegExp(match, 'g')); - return m ? m.length : 0; -} +function count(haystack: string, needle: string) { + if (!needle) return 0; + let i = 0, idx = 0; + while ((idx = haystack.indexOf(needle, idx)) !== -1) { i++; idx += needle.length; } + return i; +}Also applies to: 64-67
plugins/development/README.md (2)
26-26: Use a repo-relative link to avoid branch driftHardcoding master can go stale. Prefer a relative path.
-see [config/config.default.ts](https://github.com/eggjs/egg/blob/master/plugins/development/src/config/config.default.ts) for more detail. +see [src/config/config.default.ts](./src/config/config.default.ts) for more detail.
58-65: Fix TypeScript snippet (invalid export syntax)The example won’t compile as written.
-// config/config.default.ts -export default = { +// config/config.default.ts +export default { development: { // don't reload when css fileChanged // https://github.com/sindresorhus/multimatch reloadPattern: ['**', '!**/*.css'], }, };plugins/development/package.json (1)
69-71: Align Node engines with documented supportCHANGELOG says “>=18.19.0”, package.json requires “>=20.19.0”. Pick one baseline and align docs and engines.
If the monorepo standard is Node 20.19, update CHANGELOG accordingly; otherwise, relax engines here.
plugins/development/CHANGELOG.md (1)
11-13: Node version in changelog conflicts with package.jsonChangelog states “drop Node.js < 18.19.0”, while package.json requires “>= 20.19.0”. Please reconcile.
plugins/development/src/app/middleware/egg_loader_trace.ts (2)
10-10: Remove redundant await.
return await next()adds no value and worsens stack traces.- return await next(); + return next();
21-41: Harden timing-file loading (optional).Wrap file IO in try/catch and skip bad entries to avoid 500s when a transient/partial timing file exists.
Do you want a patch that guards
fs.readdir/JSON.parseand logs at debug level instead of throwing?CLAUDE.md (1)
190-202: Add “how to enable a plugin” snippet.Consider adding a short example showing
config/plugin.tsenabling a plugin (and noting local-only enablement fordevelopment).Example to add:
export default { development: { enable: true, package: '@eggjs/development' }, // only in local };plugins/development/src/types.ts (1)
5-9: Optional: make arrays readonly.Reduce accidental mutation of config by using
readonly string[].- watchDirs: string[]; + watchDirs: readonly string[]; @@ - ignoreDirs: string[]; + ignoreDirs: readonly string[];plugins/development/src/config/loader_trace.html (1)
4-4: Add a meaningful page title.The page title is empty, which impacts SEO and user experience. Consider adding a descriptive title.
Apply this diff to add a title:
- <title></title> + <title>Egg Loader Trace</title>plugins/development/src/app.ts (1)
6-11: Race condition risk with fastReady in constructor.Setting
app.ready(true)in the constructor vianextTickcan cause race conditions if other plugins depend on normal initialization flow.Consider moving this logic to a lifecycle method like
configDidLoadto ensure proper initialization order. Also, add a warning log when fastReady is enabled.constructor(app: EggCore) { this.#app = app; - // if true, then don't need to wait at local development mode - if (app.config.development.fastReady) { - process.nextTick(() => this.#app.ready(true)); - } } async configWillLoad() { this.#app.config.coreMiddleware.push('eggLoaderTrace'); } + + async configDidLoad() { + // if true, then don't need to wait at local development mode + if (this.#app.config.development.fastReady) { + this.#app.logger.warn('[development] fastReady is enabled, skipping normal ready wait'); + process.nextTick(() => this.#app.ready(true)); + } + }plugins/development/test/fast_ready_false.test.ts (2)
20-20: Flaky test timing with hardcoded waits.Using hardcoded wait times (100ms, 300ms) makes tests flaky and slow. These can fail on slower CI systems or pass incorrectly on faster ones.
Consider using a more robust approach like polling for expected conditions or using event-based waiting:
- // We need to wait for log written, because app.logger.info is async. - await scheduler.wait(100); + // Poll for log output with timeout + const maxWait = 5000; + const interval = 100; + let elapsed = 0; + while (elapsed < maxWait) { + try { + app.expect('stdout', /Server started./); + break; + } catch { + await scheduler.wait(interval); + elapsed += interval; + } + }Also applies to: 31-31
12-12: Clarify the purpose of the 500ms debounce wait.The comment "for debounce" doesn't explain what is being debounced or why this wait is necessary.
Update the comment to be more descriptive:
- // for debounce + // Wait for file watcher debounce to complete and prevent test interference afterEach(() => scheduler.wait(500));plugins/development/test/timing.test.ts (2)
21-24: Fragile regex-based JSON extraction.Extracting JSON using regex from HTML is fragile and can break with minor template changes.
Consider having the endpoint also expose a JSON API endpoint for testing, or use a more robust HTML parser:
+ // Consider adding a JSON endpoint for testing + // const res = await app.httpRequest().get('/__loader_trace__.json'); + // const json = res.body; + const jsonString = res.text.match(/data = (.*?);/); assert(jsonString); + assert(jsonString[1], 'JSON data not found in response'); assert(jsonString[1].length > 3000);
34-34: Remove commented debug code.Debug console.log should be removed before merging.
- // console.log(last);plugins/development/src/agent.ts (3)
24-24: Potential race condition when deleting timing files.Multiple agent processes might try to delete the same files simultaneously, causing errors.
Consider adding error handling for concurrent deletion:
- await fs.rm(path.join(rundir, file), { force: true, recursive: true }); + try { + await fs.rm(path.join(rundir, file), { force: true, recursive: true }); + } catch (err: any) { + // Ignore ENOENT errors as file might be deleted by another process + if (err.code !== 'ENOENT') { + logger.warn(`[agent:development] Failed to clean timing file ${file}: ${err.message}`); + } + }
60-63: Missing null check for process.send.While TypeScript's non-null assertion (!) is used, it's safer to explicitly check for IPC availability.
Add a safety check:
- process.send!({ + if (!process.send) { + logger.error('[agent:development] IPC channel not available, cannot reload worker'); + return; + } + process.send({ to: 'master', action: 'reload-worker', });
55-64: Consider making debounce delay configurable.The 200ms debounce delay is hardcoded. Different projects might benefit from different values.
Consider making this configurable:
+ const debounceDelay = config.reloadDebounceMs || 200; const reloadFile = debounce(function (info) { logger.warn( `[agent:development] reload worker because ${info.path} ${info.event}` ); process.send!({ to: 'master', action: 'reload-worker', }); - }, 200); + }, debounceDelay);plugins/development/test/development-ts.test.ts (2)
64-66: Harden count() against regex-from-variable.Escape the needle to avoid regex surprises/ReDoS warnings.
Apply:
-function count(str: string, match: string) { - const m = str.match(new RegExp(match, 'g')); +function count(str: string, match: string) { + const m = str.match(new RegExp(regexEscape(match), 'g'));
25-25: Safer cleanup: tolerate missing files.Use rm(..., { force: true }) to avoid throwing on ENOENT and reduce flakes on fast watchers.
Apply:
- await fs.unlink(filepath); + await fs.rm(filepath, { force: true }); @@ - await fs.unlink(filepath); + await fs.rm(filepath, { force: true }); @@ - await fs.unlink(filepath); - await fs.unlink(filepath1); + await fs.rm(filepath, { force: true }); + await fs.rm(filepath1, { force: true });Also applies to: 37-37, 57-58
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (56)
CLAUDE.md(2 hunks)packages/egg/package.json(1 hunks)plugins/development/CHANGELOG.md(1 hunks)plugins/development/LICENSE(1 hunks)plugins/development/README.md(1 hunks)plugins/development/package.json(1 hunks)plugins/development/src/agent.ts(1 hunks)plugins/development/src/app.ts(1 hunks)plugins/development/src/app/middleware/egg_loader_trace.ts(1 hunks)plugins/development/src/config/config.default.ts(1 hunks)plugins/development/src/config/loader_trace.html(1 hunks)plugins/development/src/index.ts(1 hunks)plugins/development/src/types.ts(1 hunks)plugins/development/src/utils.ts(1 hunks)plugins/development/test/absolute.test.ts(1 hunks)plugins/development/test/custom.test.ts(1 hunks)plugins/development/test/development-ts.test.ts(1 hunks)plugins/development/test/development.test.ts(1 hunks)plugins/development/test/fast_ready_false.test.ts(1 hunks)plugins/development/test/fixtures/absolute/config/config.default.js(1 hunks)plugins/development/test/fixtures/absolute/package.json(1 hunks)plugins/development/test/fixtures/custom/config/config.default.js(1 hunks)plugins/development/test/fixtures/custom/package.json(1 hunks)plugins/development/test/fixtures/delay-ready/app.js(1 hunks)plugins/development/test/fixtures/delay-ready/package.json(1 hunks)plugins/development/test/fixtures/development-ts/app/public/foo.js(1 hunks)plugins/development/test/fixtures/development-ts/app/router.ts(1 hunks)plugins/development/test/fixtures/development-ts/config/config.default.ts(1 hunks)plugins/development/test/fixtures/development-ts/package.json(1 hunks)plugins/development/test/fixtures/development-ts/tsconfig.json(1 hunks)plugins/development/test/fixtures/development/app/public/foo.js(1 hunks)plugins/development/test/fixtures/development/app/router.js(1 hunks)plugins/development/test/fixtures/development/config/config.default.js(1 hunks)plugins/development/test/fixtures/development/package.json(1 hunks)plugins/development/test/fixtures/fast-ready/app.js(1 hunks)plugins/development/test/fixtures/fast-ready/config/config.default.js(1 hunks)plugins/development/test/fixtures/fast-ready/package.json(1 hunks)plugins/development/test/fixtures/not-reload/config/config.default.js(1 hunks)plugins/development/test/fixtures/not-reload/package.json(1 hunks)plugins/development/test/fixtures/override-ignore/config/config.default.js(1 hunks)plugins/development/test/fixtures/override-ignore/package.json(1 hunks)plugins/development/test/fixtures/override/config/config.default.js(1 hunks)plugins/development/test/fixtures/override/package.json(1 hunks)plugins/development/test/fixtures/timing/app.js(1 hunks)plugins/development/test/fixtures/timing/app/router.js(1 hunks)plugins/development/test/fixtures/timing/config/config.js(1 hunks)plugins/development/test/fixtures/timing/package.json(1 hunks)plugins/development/test/not-reload.test.ts(1 hunks)plugins/development/test/override.test.ts(1 hunks)plugins/development/test/process_mode_single.test.ts(1 hunks)plugins/development/test/timing.test.ts(1 hunks)plugins/development/test/utils.ts(1 hunks)plugins/development/tsconfig.json(1 hunks)plugins/development/tsdown.config.ts(1 hunks)pnpm-workspace.yaml(4 hunks)tsconfig.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
packages/*/package.json
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use "workspace:*" for internal workspace dependencies in package.json
Files:
packages/egg/package.json
{packages,tools}/**/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
{packages,tools}/**/package.json: Use workspace:* for internal workspace dependencies
Use catalog: specifiers for external dependencies defined in pnpm-workspace.yaml catalog
Files:
packages/egg/package.json
packages/egg/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
packages/egg should expose hybrid CommonJS/ESM exports
Files:
packages/egg/package.json
pnpm-workspace.yaml
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Define dependency versions via the catalog section in pnpm-workspace.yaml and reference them with "catalog:"
Define and manage centralized dependency versions in the catalog section
Files:
pnpm-workspace.yaml
{tsconfig.json,packages/*/tsconfig.json}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable strict TypeScript mode across all packages
Files:
tsconfig.json
tsconfig.json
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Root tsconfig.json must include all packages in the references array
Files:
tsconfig.json
🧠 Learnings (21)
📓 Common learnings
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to packages/egg/src/config/{plugin.ts,config.default.ts} : Maintain built-in plugin and default configuration in packages/egg/src/config/plugin.ts and config.default.ts
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to packages/egg/src/config/{plugin.ts,config.default.ts} : Maintain built-in plugin and default configuration in packages/egg/src/config/plugin.ts and config.default.ts
Applied to files:
plugins/development/test/fixtures/custom/config/config.default.jsplugins/development/test/fixtures/development-ts/package.jsonplugins/development/tsconfig.jsonpackages/egg/package.jsonplugins/development/test/fixtures/override-ignore/config/config.default.jsplugins/development/test/fixtures/development-ts/tsconfig.jsonplugins/development/src/types.tsplugins/development/tsdown.config.tsplugins/development/src/config/config.default.tsplugins/development/test/fixtures/absolute/config/config.default.jsplugins/development/src/app.tsplugins/development/test/fixtures/development-ts/config/config.default.tspnpm-workspace.yamlplugins/development/package.jsontsconfig.jsonCLAUDE.md
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to {packages,tools}/**/tsconfig.json : Set compilerOptions.baseUrl to "./" in sub-project tsconfig.json files
Applied to files:
plugins/development/tsconfig.json
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to {packages,tools}/**/tsconfig.json : All sub-project tsconfig.json files must extend the root tsconfig.json via "extends": "../../tsconfig.json"
Applied to files:
plugins/development/tsconfig.jsonplugins/development/test/fixtures/development-ts/tsconfig.jsontsconfig.json
📚 Learning: 2025-09-14T08:41:30.606Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.606Z
Learning: Applies to packages/*/tsconfig.json : All sub-project tsconfig.json files must extend from the root via "extends": "../../tsconfig.json"
Applied to files:
plugins/development/tsconfig.jsonplugins/development/test/fixtures/development-ts/tsconfig.json
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to packages/{egg,core}/tsconfig.json : Enable TypeScript strict mode in packages/egg and packages/core
Applied to files:
plugins/development/tsconfig.jsonplugins/development/test/fixtures/development-ts/tsconfig.jsonplugins/development/test/fixtures/development-ts/config/config.default.tsplugins/development/package.jsontsconfig.json
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to tools/egg-bin/test/**/*.test.ts : egg-bin uses Mocha for tests (exception to Vitest rule)
Applied to files:
plugins/development/test/fast_ready_false.test.tsplugins/development/test/process_mode_single.test.tsplugins/development/test/custom.test.tsplugins/development/test/not-reload.test.tsplugins/development/test/development.test.tsplugins/development/test/development-ts.test.tsplugins/development/test/timing.test.tsplugins/development/test/override.test.ts
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to tools/egg-bin/package.json : tools/egg-bin should be built on oclif/core
Applied to files:
packages/egg/package.jsonpnpm-workspace.yamlplugins/development/package.jsonCLAUDE.md
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to packages/egg/package.json : packages/egg should expose hybrid CommonJS/ESM exports
Applied to files:
packages/egg/package.jsonpnpm-workspace.yamlplugins/development/package.jsonCLAUDE.md
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to {packages,tools}/**/package.json : Use workspace:* for internal workspace dependencies
Applied to files:
packages/egg/package.jsonpnpm-workspace.yamlCLAUDE.md
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to {packages,tools}/**/package.json : Use catalog: specifiers for external dependencies defined in pnpm-workspace.yaml catalog
Applied to files:
packages/egg/package.jsonpnpm-workspace.yaml
📚 Learning: 2025-09-14T08:41:30.606Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.606Z
Learning: Applies to packages/*/package.json : Use "workspace:*" for internal workspace dependencies in package.json
Applied to files:
packages/egg/package.jsonpnpm-workspace.yaml
📚 Learning: 2025-09-14T08:41:30.606Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.606Z
Learning: Applies to pnpm-workspace.yaml : Define dependency versions via the catalog section in pnpm-workspace.yaml and reference them with "catalog:"
Applied to files:
packages/egg/package.jsonpnpm-workspace.yaml
📚 Learning: 2025-09-14T08:41:30.606Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.606Z
Learning: Applies to packages/**/test/**/*.test.ts : Use Node.js built-in assert module for test assertions
Applied to files:
plugins/development/test/absolute.test.tsplugins/development/test/custom.test.ts
📚 Learning: 2025-09-14T08:41:30.606Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.606Z
Learning: Applies to {tsconfig.json,packages/*/tsconfig.json} : Enable strict TypeScript mode across all packages
Applied to files:
plugins/development/test/fixtures/development-ts/tsconfig.json
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to {packages,tools}/**/tsdown.config.ts : Each package should include tsdown.config.ts for unbundled ESM builds
Applied to files:
plugins/development/test/fixtures/development-ts/tsconfig.jsonplugins/development/tsdown.config.ts
📚 Learning: 2025-09-14T08:41:30.606Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.606Z
Learning: Applies to packages/**/src/**/*.{ts,tsx} : Use TypeScript for all source files in packages (no JavaScript source files)
Applied to files:
plugins/development/test/fixtures/development-ts/tsconfig.json
📚 Learning: 2025-09-14T08:41:30.606Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.606Z
Learning: Applies to packages/*/tsdown.config.ts : Each package must provide a tsdown.config.ts for unbundled ESM builds
Applied to files:
plugins/development/test/fixtures/development-ts/tsconfig.jsonplugins/development/tsdown.config.ts
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to pnpm-workspace.yaml : Define and manage centralized dependency versions in the catalog section
Applied to files:
pnpm-workspace.yaml
📚 Learning: 2025-09-14T08:41:30.606Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.606Z
Learning: Applies to tsconfig.json : Root tsconfig.json must include all packages in the references array
Applied to files:
tsconfig.json
📚 Learning: 2025-09-14T08:41:30.606Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.606Z
Learning: Applies to packages/egg/src/app/extend/** : Place framework extensions (context, helper, request, response) under packages/egg/src/app/extend/
Applied to files:
CLAUDE.md
🧬 Code graph analysis (19)
plugins/development/test/fixtures/development/app/router.js (1)
plugins/development/test/fixtures/development-ts/app/router.ts (1)
app(3-11)
plugins/development/test/utils.ts (1)
packages/mock/test/helper.ts (1)
__dirname(3-3)
plugins/development/test/fast_ready_false.test.ts (1)
packages/mock/src/register.ts (1)
afterEach(29-36)
plugins/development/test/fixtures/delay-ready/app.js (1)
plugins/development/test/fixtures/fast-ready/app.js (1)
done(4-4)
plugins/development/test/absolute.test.ts (1)
plugins/development/test/utils.ts (1)
getFilepath(7-9)
plugins/development/src/utils.ts (1)
packages/mock/test/helper.ts (1)
__dirname(3-3)
plugins/development/test/process_mode_single.test.ts (2)
plugins/development/test/fixtures/development-ts/app/router.ts (1)
app(3-11)plugins/development/test/utils.ts (1)
getFilepath(7-9)
plugins/development/src/types.ts (1)
packages/egg/src/lib/types.ts (1)
EggAppConfig(97-271)
plugins/development/src/config/config.default.ts (1)
plugins/development/src/types.ts (1)
DevelopmentConfig(1-30)
plugins/development/test/fixtures/fast-ready/app.js (1)
plugins/development/test/fixtures/delay-ready/app.js (1)
done(4-4)
plugins/development/test/custom.test.ts (1)
plugins/development/test/utils.ts (2)
getFilepath(7-9)DELAY(15-15)
plugins/development/test/fixtures/absolute/config/config.default.js (1)
packages/egg/src/config/config.default.ts (1)
appInfo(14-414)
plugins/development/test/not-reload.test.ts (1)
plugins/development/test/utils.ts (3)
getFilepath(7-9)DELAY(15-15)escape(11-13)
plugins/development/test/development.test.ts (1)
plugins/development/test/utils.ts (3)
getFilepath(7-9)escape(11-13)DELAY(15-15)
plugins/development/test/development-ts.test.ts (1)
plugins/development/test/utils.ts (3)
getFilepath(7-9)escape(11-13)DELAY(15-15)
plugins/development/src/agent.ts (2)
packages/core/src/egg.ts (1)
baseDir(288-290)plugins/development/src/utils.ts (1)
isTimingFile(17-19)
plugins/development/test/fixtures/development-ts/config/config.default.ts (1)
packages/egg/src/lib/types.ts (1)
EggAppConfig(97-271)
plugins/development/test/override.test.ts (1)
plugins/development/test/utils.ts (2)
getFilepath(7-9)escape(11-13)
plugins/development/src/app/middleware/egg_loader_trace.ts (1)
plugins/development/src/utils.ts (2)
getSourceFile(13-15)isTimingFile(17-19)
🪛 Biome (2.1.2)
plugins/development/test/utils.ts
[error] 11-11: Do not shadow the global "escape" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
plugins/development/test/not-reload.test.ts
[error] 4-4: Do not shadow the global "escape" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
plugins/development/test/development.test.ts
[error] 5-5: Do not shadow the global "escape" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
plugins/development/test/development-ts.test.ts
[error] 5-5: Do not shadow the global "escape" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
plugins/development/test/override.test.ts
[error] 4-4: Do not shadow the global "escape" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🪛 ast-grep (0.38.6)
plugins/development/test/development.test.ts
[warning] 64-64: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(match, 'g')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
plugins/development/test/development-ts.test.ts
[warning] 64-64: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(match, 'g')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: Test egg-bin (windows-latest, 24)
- GitHub Check: Test egg-bin (windows-latest, 22)
- GitHub Check: Test egg-bin (ubuntu-latest, 22)
- GitHub Check: Test packages (ubuntu-latest, 22)
- GitHub Check: Test packages (ubuntu-latest, 24)
- GitHub Check: Test packages (windows-latest, 24)
- GitHub Check: Test packages (windows-latest, 22)
- GitHub Check: Cloudflare Pages: egg-v3
🔇 Additional comments (30)
plugins/development/test/fixtures/development/package.json (1)
1-3: No action needed — fixture not enrolled in pnpm workspace.
pnpm-workspace.yaml uses "plugins/*" (not "plugins/**"), so plugins/development/test/fixtures/development/package.json will not be picked up by the workspace.packages/egg/package.json (1)
121-121: Correct: switch @eggjs/development to workspaceThis aligns with the monorepo move and workspace policy.
pnpm-workspace.yaml (3)
3-3: Workspace packages: inclusion of plugins/ looks good.*
3-3: Confirmed — @eggjs/development is workspace-sourced and not pinned in catalog. No pnpm-workspace.yaml entry for @eggjs/development; packages/egg/package.json lists '@eggjs/development': 'workspace:*'.
76-76: No action required — packages already use catalog: specifiers.
Scan of package.json files for debounce, multimatch, and eslint-config-egg returned "No offenders found."plugins/development/tsconfig.json (1)
1-6: Extends + baseUrl are correct; verify strict mode is enabled via the root or set locally.If strict isn’t inherited from ../../tsconfig.json (via @eggjs/tsconfig), add:
{ "extends": "../../tsconfig.json", "compilerOptions": { - "baseUrl": "./" + "baseUrl": "./", + "strict": true } }plugins/development/test/fixtures/timing/config/config.js (1)
1-3: LGTM.tsconfig.json (1)
37-39: Reference to plugins/development added correctly.plugins/development/test/fixtures/development-ts/app/public/foo.js (1)
1-1: LGTM.plugins/development/LICENSE (1)
1-21: MIT license text looks correct and complete.plugins/development/test/fixtures/delay-ready/app.js (1)
3-13: Readiness flow fixture LGTM.Clear signal with
readyCallbackand log; no issues.plugins/development/test/fixtures/not-reload/config/config.default.js (1)
3-5: Config shape LGTM — type & docs verified.
DevelopmentConfig declares reloadOnDebug:boolean (plugins/development/src/types.ts); default/docs include the option (plugins/development/src/config/config.default.ts); agent uses it (plugins/development/src/agent.ts) and the fixture sets reloadOnDebug:false — approve.plugins/development/test/fixtures/override-ignore/config/config.default.js (1)
3-6: Override ignore defaults for public dir — Verified.
plugins/development/src/agent.ts clears the built‑in ignores when config.overrideIgnore is true and then concatenates config.ignoreDirs, so overrideIgnore precedence is honored (see plugins/development/src/agent.ts lines 47–53).plugins/development/test/fixtures/development/app/public/foo.js (1)
1-1: Ensure test fixtures are excluded from publish.Verified: plugins/development/package.json "files" = ["dist"] and no .npmignore found — test/fixtures/** will not be included in the published package.
plugins/development/test/fixtures/absolute/config/config.default.js (1)
5-11: LGTM for absolute watchDirs fixtureConfig is valid for the absolute-path watch scenario; no issues.
plugins/development/test/fixtures/override/config/config.default.js (1)
3-6: LGTM: scoped development override for testsMatches the fixture’s intent and minimal surface area.
plugins/development/test/fixtures/timing/app.js (1)
4-15: LGTM: simple FS probes for timing/config filesStraightforward fixture logic; no changes needed.
plugins/development/test/fixtures/development-ts/app/router.ts (1)
3-11: Use router API; TypeScript Application may not expose get()Type defs for Application typically expose router, not get(); switch to app.router.get to satisfy TS and align with Egg idioms.
-export default (app: Application) => { - app.get('/foo.js', async ctx => { +export default (app: Application) => { + app.router.get('/foo.js', async ctx => { ctx.body = 'foo.js'; }); - app.get('/foo', async ctx => { + app.router.get('/foo', async ctx => { ctx.body = 'foo'; }); };Confirm your local TypeScript compile currently passes; if not, the diff above should fix it.
plugins/development/package.json (1)
72-76: No action required — oxlint is declared in plugins/development/package.json
"oxlint": "catalog:" exists at line 21 of that package.json, so the lint script is covered.CLAUDE.md (1)
39-43: Nice: clear plugins/ structure guidance.The plugins/ consolidation and local-only note for development looks good.
plugins/development/src/index.ts (1)
1-1: LGTM: side‑effect import ensures type augmentation is loaded.No further action.
plugins/development/test/fixtures/fast-ready/package.json (1)
1-3: LGTM: minimal fixture manifest.Sufficient for test discovery.
plugins/development/test/fixtures/fast-ready/config/config.default.js (1)
3-5: LGTM: targeted override for fastReady.Matches the intended test behavior.
plugins/development/test/fixtures/fast-ready/app.js (1)
3-13: LGTM: clear readiness choreography for the fixture.Logs and callback timing are appropriate.
plugins/development/test/fixtures/development-ts/config/config.default.ts (1)
1-10: LGTM — NodeNext moduleResolution verified. TS fixture wires augmentation and config typing; casting to EggAppConfig is correct. tests tsconfig (plugins/development/test/fixtures/development-ts/tsconfig.json:8) sets "moduleResolution": "NodeNext", so the '../../../../src/index.js' import will resolve to src/index.ts under NodeNext resolution.plugins/development/src/config/config.default.ts (1)
13-23: LGTM! Well-structured default configuration.The default configuration is properly typed and provides sensible defaults for the development plugin. The use of
as DevelopmentConfigensures type safety.plugins/development/src/agent.ts (1)
78-96: Verify agent.watcher exists before useAdd a guard in plugins/development/src/agent.ts (lines 78–96) before calling agent.watcher.watch — e.g. if (!agent.watcher) return; or use agent.watcher?.watch(...). Watcher initialization not found in the repo; confirm it's always initialized.
plugins/development/test/development-ts.test.ts (3)
9-15: Good test bootstrap.Per-test local env + fresh cluster and awaitable ready() look solid.
5-5: Fix lint error: alias imported "escape" to avoid shadowing the global.Biome flags
escapeas shadowing the global — alias it (e.g.escape as regexEscape) and update usages (new RegExp(escape(...))→new RegExp(regexEscape(...))) in these files:
- plugins/development/test/override.test.ts (import at line 4)
- plugins/development/test/development.test.ts (import at line 5)
- plugins/development/test/development-ts.test.ts (import at line 5)
- plugins/development/test/not-reload.test.ts (import at line 4)
33-43: Negative assertion OK — app/assets is ignored by watcher; test is valid.plugins/development/src/agent.ts defaults ignoreReloadFileDirs to include "app/assets" (resolved against baseDir) and the watcher returns early when isAssetsDir(info.path) is true (uses startsWith), so changes under app/assets won't trigger reload.
| export function escape(str: string) { | ||
| return str.replace(/[|\\{}()[\]^$+*?.]/g, '\\$&').replace(/-/g, '\\x2d'); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Rename escape() to avoid shadowing the global API (Biome error)
Fixes lint/suspicious/noShadowRestrictedNames and improves clarity.
-export function escape(str: string) {
- return str.replace(/[|\\{}()[\]^$+*?.]/g, '\\$&').replace(/-/g, '\\x2d');
-}
+export function escapeRegExp(str: string) {
+ return str.replace(/[|\\{}()[\]^$+*?.]/g, '\\$&').replace(/-/g, '\\x2d');
+}Update imports across tests. Run:
Then replace imports/usages with escapeRegExp.
🏁 Script executed:
#!/bin/bash
rg -nP "from './utils\.js';?$" -C0
rg -nP "\bescape\(" -g 'plugins/development/test/**' -SLength of output: 1621
Rename exported escape() → escapeRegExp and update imports/usages
Avoid Biome lint suspicious/noShadowRestrictedNames; rename the exported function and update all imports/call sites.
-export function escape(str: string) {
- return str.replace(/[|\\{}()[\]^$+*?.]/g, '\\$&').replace(/-/g, '\\x2d');
-}
+export function escapeRegExp(str: string) {
+ return str.replace(/[|\\{}()[\]^$+*?.]/g, '\\$&').replace(/-/g, '\\x2d');
+}Files to update:
- Change export in: plugins/development/test/utils.ts (line 11).
- Update imports/usages:
- plugins/development/test/override.test.ts — import at line 4; usages at lines 60, 73.
- plugins/development/test/not-reload.test.ts — import at line 4; usage at line 32.
- plugins/development/test/development-ts.test.ts — import at line 5; usages at lines 29, 41.
- plugins/development/test/development.test.ts — import at line 5; usages at lines 29, 41.
Replace import specifiers (escape → escapeRegExp) and call sites (escape(...) → escapeRegExp(...)).
📝 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.
| export function escape(str: string) { | |
| return str.replace(/[|\\{}()[\]^$+*?.]/g, '\\$&').replace(/-/g, '\\x2d'); | |
| } | |
| export function escapeRegExp(str: string) { | |
| return str.replace(/[|\\{}()[\]^$+*?.]/g, '\\$&').replace(/-/g, '\\x2d'); | |
| } |
🧰 Tools
🪛 Biome (2.1.2)
[error] 11-11: Do not shadow the global "escape" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🤖 Prompt for AI Agents
In plugins/development/test/utils.ts around lines 11-13, the exported function
escape should be renamed to escapeRegExp to avoid Biome lint
suspicious/noShadowRestrictedNames; change the export identifier accordingly.
Then update all imports and call sites to use escapeRegExp: in
plugins/development/test/override.test.ts update the import at line 4 and
replace usages at lines 60 and 73; in
plugins/development/test/not-reload.test.ts update the import at line 4 and
replace the usage at line 32; in plugins/development/test/development-ts.test.ts
update the import at line 5 and replace usages at lines 29 and 41; and in
plugins/development/test/development.test.ts update the import at line 5 and
replace usages at lines 29 and 41. Ensure the new exported name is exported as
the same function implementation and run tests to verify no missing imports.
| export default defineConfig({ | ||
| entry: 'src/**/*.ts', | ||
| unbundle: true, | ||
| dts: true, | ||
| exports: { | ||
| devExports: true, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Potential missing static assets in build.
entry: 'src/**/*.ts' won’t copy non‑TS assets like src/config/loader_trace.html that the plugin serves. Ensure the build/publish includes these files, or embed them.
Proposed tsdown tweak (if supported):
export default defineConfig({
entry: 'src/**/*.ts',
unbundle: true,
dts: true,
+ // Ensure static assets are copied to dist (adjust if your tsdown version uses a different key)
+ assets: [
+ { from: 'src/config/loader_trace.html', to: 'dist/config/loader_trace.html' },
+ ],
exports: {
devExports: true,
},
});🏁 Script executed:
#!/bin/bash
# Locate HTML asset and references
fd loader_trace.html
rg -n "loader_trace\.html" -C2 plugins/development/src
# Check if tsdown config or package scripts copy assets
rg -n 'assets|copy|cpx|cp -r|postbuild' -S plugins/development
# After a local build (outside this sandbox), verify the file exists in dist:
# ls -l plugins/development/dist/config/loader_trace.htmlLength of output: 1926
Include loader_trace.html in build/publish
plugins/development/src/config/loader_trace.html is read at runtime by plugins/development/src/app/middleware/egg_loader_trace.ts (fs.readFile(getSourceFile('config/loader_trace.html'))), but plugins/development/tsdown.config.ts only bundles TypeScript (entry: 'src/**/*.ts') so the asset will not be copied. Ensure the build/publish copies the file into dist (e.g., dist/config/loader_trace.html) or include it in the package (postbuild copy or package.json "files").
Locations: plugins/development/tsdown.config.ts; plugins/development/src/config/loader_trace.html; plugins/development/src/app/middleware/egg_loader_trace.ts:13.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #5490 +/- ##
==========================================
- Coverage 76.24% 76.20% -0.05%
==========================================
Files 104 108 +4
Lines 5696 5774 +78
Branches 1195 1215 +20
==========================================
+ Hits 4343 4400 +57
- Misses 1183 1199 +16
- Partials 170 175 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (8)
plugins/development/test/absolute.test.ts (1)
35-35: Remove debugging log from tests.Keeps CI output clean.
- console.log(`write file to ${filepath}`);plugins/development/test/development-ts.test.ts (3)
48-51: Title vs. expectation mismatch and CI skip.Either rename the test to say “reload twice…” if expecting 2 events, or assert 1. Also prefer it.skip in CI to make intent explicit.
-it('should reload once when 2 file change', async () => { +it.skipIf(!!process.env.CI)('should reload twice when two file changes happen in separate batches', async () => { - if (process.env.CI) { - return; - }
8-8: Update import to renamed helper.-import { escape, getFilepath, DELAY } from './utils.ts'; +import { escapeRegExp, getFilepath, DELAY } from './utils.ts';And below:
- new RegExp(escape(`reload worker because ${filepath}`)) + new RegExp(escapeRegExp(`reload worker because ${filepath}`))
22-22: Use DELAY instead of magic waits.- afterEach(() => scheduler.wait(500)); + afterEach(() => scheduler.wait(DELAY / 10)); @@ - await scheduler.wait(1000); + await scheduler.wait(DELAY / 5); @@ - await scheduler.wait(5000); + await scheduler.wait(DELAY); @@ - await scheduler.wait(1000); + await scheduler.wait(DELAY / 5); @@ - await scheduler.wait(5000); + await scheduler.wait(DELAY);Also applies to: 27-29, 39-41
plugins/development/test/utils.ts (1)
11-13: Rename escape() to avoid shadowing the global API (Biome error).Use a clearer name and update call sites.
Apply:
-export function escape(str: string) { +export function escapeRegExp(str: string) { return str.replace(/[|\\{}()[\]^$+*?.]/g, '\\$&').replace(/-/g, '\\x2d'); }Then replace imports/usages in tests accordingly (see suggested diffs in those files).
plugins/development/test/not-reload.test.ts (1)
6-6: Update import to renamed helper.Follow the utils rename to escapeRegExp.
-import { escape, getFilepath, DELAY } from './utils.ts'; +import { escapeRegExp, getFilepath, DELAY } from './utils.ts';And update usage:
- new RegExp(escape(`reload worker because ${filepath} change`)) + new RegExp(escapeRegExp(`reload worker because ${filepath} change`))plugins/development/test/override.test.ts (1)
6-6: Update import to renamed helper.Track the utils rename to escapeRegExp.
-import { escape, getFilepath } from './utils.ts'; +import { escapeRegExp, getFilepath, DELAY } from './utils.ts';plugins/development/test/development.test.ts (1)
48-50: Test skipped on CI – make it deterministic rather than environment‑gated.Prefer event/promise-based waiting over fixed delays so this can run on CI reliably.
I can help refactor this test to wait on a reload signal/log stream with a timeout instead of
process.env.CIchecks.
🧹 Nitpick comments (9)
plugins/development/test/fixtures/development-process_mode_single/app/router.js (1)
1-9: Prefer ctx over this in handlers (nit).Using ctx makes intent explicit and avoids dependence on function binding.
Apply if you want consistency with modern Egg/Koa style:
module.exports = app => { - app.get('/foo.js', async function () { - this.body = 'foo.js'; - }); + app.get('/foo.js', async ctx => { + ctx.body = 'foo.js'; + }); - app.get('/foo', async function () { - this.body = 'foo'; - }); + app.get('/foo', async ctx => { + ctx.body = 'foo'; + }); };plugins/development/test/utils.ts (2)
4-6: Optional: use import.meta.dirname when Node version allows.Node 20+ exposes import.meta.dirname, simplifying this helper.
Apply:
-const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const __dirname = (import.meta as any).dirname ?? path.dirname(fileURLToPath(import.meta.url));Or, if the repo baseline is Node ≥ 20.10, use import.meta.dirname directly.
15-15: Consider parameterizing DELAY via env for local tuning.Expose DELAY_MULTIPLIER (default 1) to shorten/extend waits locally without editing code.
-export const DELAY = process.env.CI ? 30000 : 5500; +const BASE_DELAY = process.env.CI ? 30000 : 5500; +export const DELAY = BASE_DELAY * Number(process.env.DELAY_MULTIPLIER ?? 1);plugins/development/test/not-reload.test.ts (1)
16-17: Avoid fixed inspector port to prevent CI port conflicts.Use --inspect=0 to bind a random port.
- execArgv: ['--inspect'], + execArgv: ['--inspect=0'],plugins/development/src/app/middleware/loader_trace.html (1)
3-5: Minor HTML hygiene.Add charset and a descriptive title for better defaults.
- <head> - <title></title> - </head> + <head> + <meta charset="utf-8" /> + <title>Egg Loader Trace</title> + </head>plugins/development/test/development-ts.test.ts (1)
67-69: Avoid dynamic RegExp for counting occurrences.Safer and faster to split on a known substring.
-function count(str: string, match: string) { - const m = str.match(new RegExp(match, 'g')); - return m ? m.length : 0; -} +function count(haystack: string, needle: string) { + if (!needle) return 0; + return haystack.split(needle).length - 1; +}plugins/development/test/development.test.ts (2)
8-8: Rename localescapeto avoid shadowing the global and fix Biome error.Alias the import and update its usages.
-import { escape, getFilepath, DELAY } from './utils.ts'; +import { escape as escapeRegex, getFilepath, DELAY } from './utils.ts'; @@ - new RegExp(escape(`reload worker because ${filepath}`)) + new RegExp(escapeRegex(`reload worker because ${filepath}`)) @@ - new RegExp(escape(`reload worker because ${filepath}`)) + new RegExp(escapeRegex(`reload worker because ${filepath}`))Also applies to: 29-33, 41-44
66-69: Avoid constructing a RegExp from variable input; count literal substrings instead.Prevents potential ReDoS and matches intent.
-function count(str: string, match: string) { - const m = str.match(new RegExp(match, 'g')); - return m ? m.length : 0; -} +function count(haystack: string, needle: string) { + if (!needle) return 0; + return haystack.split(needle).length - 1; +}CLAUDE.md (1)
221-227: Fix minor markdown formatting typos.Clean up stray backticks/asterisks for proper rendering.
-This configuration ensures: - -- **`entry: 'src/**/\*.ts'`\*\* - Processes all TypeScript files in src directory +This configuration ensures: + +- **entry: 'src/**/*.ts'** - Processes all TypeScript files in src directory - **`unbundle: true`** - Creates unbundled output (preserves file structure) - **`dts: true`** - Generates TypeScript declaration files - **`exports.devExports: true`** - Enables development-friendly exports
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (27)
CLAUDE.md(2 hunks)plugins/development/package.json(1 hunks)plugins/development/src/agent.ts(1 hunks)plugins/development/src/app.ts(1 hunks)plugins/development/src/app/middleware/egg_loader_trace.ts(1 hunks)plugins/development/src/app/middleware/loader_trace.html(1 hunks)plugins/development/src/config/config.default.ts(1 hunks)plugins/development/src/index.ts(1 hunks)plugins/development/src/types.ts(1 hunks)plugins/development/src/utils.ts(1 hunks)plugins/development/test/absolute.test.ts(1 hunks)plugins/development/test/custom.test.ts(1 hunks)plugins/development/test/development-ts.test.ts(1 hunks)plugins/development/test/development.test.ts(1 hunks)plugins/development/test/fast_ready_false.test.ts(1 hunks)plugins/development/test/fixtures/development-process_mode_single/app/public/foo.js(1 hunks)plugins/development/test/fixtures/development-process_mode_single/app/router.js(1 hunks)plugins/development/test/fixtures/development-process_mode_single/config/config.default.js(1 hunks)plugins/development/test/fixtures/development-process_mode_single/package.json(1 hunks)plugins/development/test/fixtures/development-ts/config/config.default.ts(1 hunks)plugins/development/test/not-reload.test.ts(1 hunks)plugins/development/test/override.test.ts(1 hunks)plugins/development/test/process_mode_single.test.ts(1 hunks)plugins/development/test/timing.test.ts(1 hunks)plugins/development/test/utils.ts(1 hunks)plugins/development/vitest.config.ts(1 hunks)vitest.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- plugins/development/test/fixtures/development-process_mode_single/package.json
🚧 Files skipped from review as they are similar to previous changes (13)
- plugins/development/src/utils.ts
- plugins/development/test/custom.test.ts
- plugins/development/test/process_mode_single.test.ts
- plugins/development/src/types.ts
- plugins/development/src/index.ts
- plugins/development/src/app.ts
- plugins/development/test/timing.test.ts
- plugins/development/src/config/config.default.ts
- plugins/development/test/fixtures/development-ts/config/config.default.ts
- plugins/development/package.json
- plugins/development/src/agent.ts
- plugins/development/test/fast_ready_false.test.ts
- plugins/development/src/app/middleware/egg_loader_trace.ts
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to packages/egg/src/config/{plugin.ts,config.default.ts} : Maintain built-in plugin and default configuration in packages/egg/src/config/plugin.ts and config.default.ts
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to packages/**/vitest.config.ts : Each non-tool package should include a vitest.config.ts
Applied to files:
plugins/development/vitest.config.tsvitest.config.ts
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to packages/**/test/**/*.test.ts : Vitest tests should import from vitest (e.g., import { describe, it } from 'vitest')
Applied to files:
plugins/development/vitest.config.tsvitest.config.tsplugins/development/test/development.test.tsplugins/development/test/absolute.test.tsplugins/development/test/not-reload.test.tsplugins/development/test/development-ts.test.tsplugins/development/test/override.test.ts
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to packages/**/test/**/*.test.ts : All new packages must use Vitest as the test runner
Applied to files:
plugins/development/vitest.config.tsvitest.config.ts
📚 Learning: 2025-09-14T08:41:30.606Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.606Z
Learning: Applies to packages/**/test/**/*.test.ts : Name test files as test/**/*.test.ts and run them with Vitest
Applied to files:
plugins/development/vitest.config.tsvitest.config.tsplugins/development/test/absolute.test.tsplugins/development/test/development-ts.test.tsplugins/development/test/override.test.ts
📚 Learning: 2025-09-14T08:41:30.606Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.606Z
Learning: Applies to packages/**/test/**/*.test.ts : Use import { describe, it } from 'vitest' in tests
Applied to files:
plugins/development/vitest.config.tsvitest.config.tsplugins/development/test/absolute.test.tsplugins/development/test/development-ts.test.tsplugins/development/test/override.test.ts
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to tools/egg-bin/test/**/*.test.ts : egg-bin uses Mocha for tests (exception to Vitest rule)
Applied to files:
plugins/development/vitest.config.tsplugins/development/test/development.test.tsplugins/development/test/absolute.test.tsplugins/development/test/not-reload.test.tsplugins/development/test/development-ts.test.tsplugins/development/test/override.test.ts
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to {packages,tools}/**/tsconfig.json : All sub-project tsconfig.json files must extend the root tsconfig.json via "extends": "../../tsconfig.json"
Applied to files:
vitest.config.ts
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to {packages,tools}/**/test/**/*.test.ts : Test files should follow the naming pattern test/**/*.test.ts
Applied to files:
vitest.config.ts
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to packages/egg/src/config/{plugin.ts,config.default.ts} : Maintain built-in plugin and default configuration in packages/egg/src/config/plugin.ts and config.default.ts
Applied to files:
CLAUDE.mdplugins/development/test/development-ts.test.tsplugins/development/test/override.test.ts
📚 Learning: 2025-09-14T08:41:30.606Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.606Z
Learning: Applies to packages/egg/src/app/extend/** : Place framework extensions (context, helper, request, response) under packages/egg/src/app/extend/
Applied to files:
CLAUDE.md
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to packages/egg/package.json : packages/egg should expose hybrid CommonJS/ESM exports
Applied to files:
CLAUDE.md
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to tools/egg-bin/package.json : tools/egg-bin should be built on oclif/core
Applied to files:
CLAUDE.md
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to {packages,tools}/**/package.json : Use workspace:* for internal workspace dependencies
Applied to files:
CLAUDE.md
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to packages/{egg,core}/tsconfig.json : Enable TypeScript strict mode in packages/egg and packages/core
Applied to files:
CLAUDE.md
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to {packages,tools}/**/tsdown.config.ts : Each package should include tsdown.config.ts for unbundled ESM builds
Applied to files:
CLAUDE.md
📚 Learning: 2025-09-14T08:41:30.606Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.606Z
Learning: Applies to packages/**/test/**/*.test.ts : Use Node.js built-in assert module for test assertions
Applied to files:
plugins/development/test/absolute.test.tsplugins/development/test/development-ts.test.tsplugins/development/test/override.test.ts
🧬 Code graph analysis (6)
plugins/development/test/development.test.ts (1)
plugins/development/test/utils.ts (3)
getFilepath(7-9)escape(11-13)DELAY(15-15)
plugins/development/test/absolute.test.ts (2)
packages/mock/src/register.ts (3)
beforeAll(22-28)afterAll(37-45)afterEach(29-36)plugins/development/test/utils.ts (1)
getFilepath(7-9)
plugins/development/test/not-reload.test.ts (1)
plugins/development/test/utils.ts (3)
getFilepath(7-9)DELAY(15-15)escape(11-13)
plugins/development/test/development-ts.test.ts (1)
plugins/development/test/utils.ts (3)
getFilepath(7-9)escape(11-13)DELAY(15-15)
plugins/development/test/override.test.ts (2)
packages/mock/src/register.ts (3)
afterAll(37-45)afterEach(29-36)beforeAll(22-28)plugins/development/test/utils.ts (2)
getFilepath(7-9)escape(11-13)
plugins/development/test/utils.ts (1)
packages/mock/test/helper.ts (1)
__dirname(3-3)
🪛 ast-grep (0.38.6)
plugins/development/test/development.test.ts
[warning] 66-66: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(match, 'g')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
plugins/development/test/development-ts.test.ts
[warning] 67-67: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(match, 'g')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🪛 Biome (2.1.2)
plugins/development/test/development.test.ts
[error] 8-8: Do not shadow the global "escape" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
plugins/development/test/not-reload.test.ts
[error] 6-6: Do not shadow the global "escape" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
plugins/development/test/development-ts.test.ts
[error] 8-8: Do not shadow the global "escape" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
plugins/development/test/override.test.ts
[error] 6-6: Do not shadow the global "escape" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
plugins/development/test/utils.ts
[error] 11-11: Do not shadow the global "escape" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: Test egg-bin (macos-latest, 22)
- GitHub Check: Test egg-bin (ubuntu-latest, 24)
- GitHub Check: Test egg-bin (ubuntu-latest, 22)
- GitHub Check: Test egg-bin (windows-latest, 24)
- GitHub Check: Test egg-bin (windows-latest, 22)
- GitHub Check: Test egg-bin (macos-latest, 24)
- GitHub Check: Test packages (windows-latest, 22)
- GitHub Check: Test packages (windows-latest, 24)
- GitHub Check: Test packages (macos-latest, 24)
- GitHub Check: Test packages (macos-latest, 22)
- GitHub Check: Test packages (ubuntu-latest, 24)
- GitHub Check: Test packages (ubuntu-latest, 22)
🔇 Additional comments (7)
plugins/development/test/fixtures/development-process_mode_single/config/config.default.js (1)
1-1: LGTM for fixture config.Matches existing fixture style; suitable for tests.
plugins/development/test/fixtures/development-process_mode_single/app/public/foo.js (1)
1-1: LGTM for test asset.Straightforward placeholder; fine for fixture use.
vitest.config.ts (1)
6-9: Root Vitest projects now include plugins — looks good.Adding 'plugins/*' ensures plugin tests are picked up without duplicating runs. The FIXME block is correctly disabled to avoid double execution.
plugins/development/vitest.config.ts (1)
3-14: Per‑project Vitest config is correct and scoped.Timeout, include/exclude patterns, and project export via defineProject are appropriate for these integration tests.
CLAUDE.md (2)
39-43: Plugins directory addition looks good.Clear placement and scope for the development plugin.
180-188: Guidance update looks consistent with the new monorepo layout.Explicitly calling out
plugins/here is helpful.plugins/development/test/development.test.ts (1)
14-18: Stabilize expectations by pinning worker count.assert(count(app.stdout, 'reload worker') >= 3) assumes a specific number of workers — set workers explicitly when creating the cluster (file: plugins/development/test/development.test.ts, ~lines 14–18).
- app = mm.cluster({ - baseDir: getFilepath('development'), - }); + app = mm.cluster({ + baseDir: getFilepath('development'), + workers: 2, + });Confirm mm.cluster in this repo supports the workers option before merging.
| </head> | ||
| <body> | ||
| <div id="mountNode"></div> | ||
| <script src="https://gw.alipayobjects.com/os/antv/assets/g2/3.0.9/g2.min.js"></script> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
External CDN without integrity.
Add SRI and consider pinning via a trusted CDN/mirror or vendoring for offline/dev use.
🌐 Web query:
What is the latest stable version of AntV G2, and does it provide an official SRI hash or CDN link we can pin to?
💡 Result:
-
Latest stable version: 5.3.5 (current stable release as of today, Sept 14, 2025). [1][2]
-
Official CDN: the G2 docs show an official CDN URL pattern (example): https://gw.alipayobjects.com/os/lib/antv/g2/{version}/dist/g2.min.js. You can also fetch packages from public CDNs (jsDelivr, unpkg, etc.). [3][1]
-
SRI / pinned integrity: AntV does not publish an official SRI hash for the CDN bundle. Public CDNs (jsDelivr, unpkg) likewise don’t provide an AntV-published SRI to pin; to use SRI you’ll need to fetch the exact file yourself, compute the hash, and pin that hash in your app. [1][3]
Sources
- @antv/g2 package metadata / docs and recent releases (npm/jsdocs listings). [1][2]
- G2 “Get started / browser” docs showing official CDN pattern. [3]
Add SRI and pin/vendor AntV G2 (CDN script unpinned)
plugins/development/src/app/middleware/loader_trace.html:8
<script src="https://gw.alipayobjects.com/os/antv/assets/g2/3.0.9/g2.min.js"></script>AntV does not publish official SRI; latest stable is 5.3.5 (Sep 14, 2025). Vendor the bundle or pin an exact released version and compute & add integrity="sha384-" plus crossorigin="anonymous", or host from a trusted CDN/mirror you control.
🤖 Prompt for AI Agents
In plugins/development/src/app/middleware/loader_trace.html around line 8, the
external AntV G2 script is unpinned and lacks an SRI/crossorigin attributes;
either vendor the bundle into the repo or pin an exact released version and add
SRI and crossorigin. Fix by downloading the exact released g2.min.js you intend
to use into the repo (or point to a trusted CDN/mirror with an exact versioned
URL), compute the SHA384 SRI hash for that exact file, update the <script> tag
to the local/versioned URL and add integrity="sha384-<computed-hash>" and
crossorigin="anonymous"; ensure the file URL and hash match the deployed
artifact so browsers can validate the resource.
| <div id="mountNode"></div> | ||
| <script src="https://gw.alipayobjects.com/os/antv/assets/g2/3.0.9/g2.min.js"></script> | ||
| <script> | ||
| var data = {{placeholder}}; |
There was a problem hiding this comment.
Unsafe templating into a script context (XSS risk).
Interpolating raw data into “var data = {{placeholder}};” is unsafe if any field contains "</script>" or HTML. Prefer JSON-in-script with textContent and parse at runtime.
Apply:
- var data = {{placeholder}};
+ // Data injected safely via JSON script tag
+ const data = JSON.parse(document.getElementById('data-json').textContent);And add a safe JSON container before this script:
- <script>
+ <script id="data-json" type="application/json">{{placeholder}}</script>
+ <script>Ensure the server renders placeholder as JSON.stringify(data) without surrounding <script> tags.
| await scheduler.wait(1000); | ||
| await fs.unlink(filepath); | ||
| await scheduler.wait(5000); | ||
| app.expect('stdout', /a\.js/); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace hard-coded waits with DELAY for stability.
Standardize timing to reduce flakes across environments.
- await scheduler.wait(1000);
+ await scheduler.wait(DELAY / 5);
@@
- await scheduler.wait(5000);
+ await scheduler.wait(DELAY);
@@
- await scheduler.wait(1000);
+ await scheduler.wait(DELAY / 5);
@@
- await scheduler.wait(5000);
+ await scheduler.wait(DELAY);And update regex helper usage:
- new RegExp(escape(`reload worker because ${filepath}`))
+ new RegExp(escapeRegExp(`reload worker because ${filepath}`))
@@
- new RegExp(escape(`reload worker because ${filepath} change`))
+ new RegExp(escapeRegExp(`reload worker because ${filepath} change`))Also applies to: 37-43, 56-62, 71-79
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (5)
plugins/development/test/not-reload.test.ts (1)
6-6: Use escapeRegExp instead of escape (Biome error, global shadowing).Replace the helper to satisfy lint and avoid shadowing the global
escape. Ensureplugins/development/test/utils.tsexportsescapeRegExp(rename or alias).Apply this diff:
-import { escape, getFilepath, DELAY } from './utils.ts'; +import { escapeRegExp, getFilepath, DELAY } from './utils.ts'; @@ - new RegExp(escape(`reload worker because ${filepath} change`)) + new RegExp(escapeRegExp(`reload worker because ${filepath} change`))Run to confirm no remaining usages:
#!/bin/bash # Find lingering imports/usages under plugins/development/test rg -nP "import\\s*\\{[^}]*\\bescape\\b" plugins/development/test --type ts -C0 rg -nP "\\bescape\\(" plugins/development/test --type ts -nAlso applies to: 31-31
plugins/development/test/override.test.ts (2)
8-8: Replace hard-coded waits with shared DELAY constants.Stabilizes timings across environments and reduces flakiness. (Matches earlier bot suggestion.)
+const DELAY = Number(process.env.EGG_DEV_DELAY ?? '') || 5000; +const BRIEF = Math.max(200, Math.floor(DELAY / 5)); @@ - await scheduler.wait(1000); + await scheduler.wait(BRIEF); @@ - await scheduler.wait(5000); + await scheduler.wait(DELAY); @@ - await scheduler.wait(1000); + await scheduler.wait(BRIEF); @@ - await scheduler.wait(5000); + await scheduler.wait(DELAY); @@ - await scheduler.wait(1000); + await scheduler.wait(BRIEF); @@ - await scheduler.wait(5000); + await scheduler.wait(DELAY); @@ - await scheduler.wait(1000); + await scheduler.wait(BRIEF); @@ - await scheduler.wait(5000); + await scheduler.wait(DELAY);Also applies to: 24-26, 34-36, 56-58, 69-71
5-7: Rename imported escape to avoid shadowing and drop .ts extension.Fix Biome’s noShadowRestrictedNames and make intent clear.
-import { beforeAll, afterAll, it, describe } from 'vitest'; -import { escape, getFilepath } from './utils.ts'; +import { beforeAll, afterAll, it, describe } from 'vitest'; +import { escape as escapeRegExp, getFilepath } from './utils'; @@ - new RegExp(escape(`reload worker because ${filepath}`)) + new RegExp(escapeRegExp(`reload worker because ${filepath}`)) @@ - new RegExp(escape(`reload worker because ${filepath} change`)) + new RegExp(escapeRegExp(`reload worker because ${filepath} change`))Also applies to: 59-62, 72-75
plugins/development/test/development-ts.test.ts (2)
49-51: Don’t skip on CI; fix timing instead.Early-returning on CI hides flakes. Prefer deterministic waits tied to DELAY and assertions based on batched changes.
22-22: Replace magic waits with DELAY to reduce flakiness.Use the shared DELAY constant instead of hard-coded millisecond values.
- afterEach(() => scheduler.wait(500)); + afterEach(() => scheduler.wait(DELAY / 10)); @@ - await scheduler.wait(1000); + await scheduler.wait(DELAY / 5); @@ - await scheduler.wait(5000); + await scheduler.wait(DELAY); @@ - await scheduler.wait(1000); + await scheduler.wait(DELAY / 5); @@ - await scheduler.wait(5000); + await scheduler.wait(DELAY);Run to find other hard-coded waits:
#!/bin/bash rg -nP "scheduler\.wait\(\s*\d+\s*\)" plugins/development/test -C1Also applies to: 27-27, 29-29, 39-39, 41-41
🧹 Nitpick comments (6)
plugins/development/test/not-reload.test.ts (1)
23-33: Guarantee fixture cleanup on assertion failure.Wrap file ops in try/finally so the temp file is removed even if the assertion throws.
it('should not reload', async () => { const filepath = getFilepath('not-reload/app/service/a.js'); - await fs.writeFile(filepath, ''); - await scheduler.wait(DELAY); - - await fs.rm(filepath, { force: true }); - app.notExpect( - 'stdout', - new RegExp(escapeRegExp(`reload worker because ${filepath} change`)) - ); + await fs.writeFile(filepath, ''); + try { + await scheduler.wait(DELAY); + app.notExpect( + 'stdout', + new RegExp(escapeRegExp(`reload worker because ${filepath} change`)) + ); + } finally { + await fs.rm(filepath, { force: true }); + } });plugins/development/test/override.test.ts (2)
27-27: Tighten the assertion to the exact reload message.Reduces false positives.
- app.expect('stdout', /a\.js/); + app.expect('stdout', new RegExp(escapeRegExp(`reload worker because ${filepath}`)));
31-31: Remove redundant app.debug() calls inside tests.Already enabled in beforeAll().
- app.debug(); + // debug already enabled in beforeAll()Also applies to: 66-66
plugins/development/test/development-ts.test.ts (3)
8-8: Avoid shadowing global escape; alias the import.Biome warns on using “escape” (global name). Alias to clarify intent and silence lint.
-import { escape, getFilepath, DELAY } from './utils.ts'; +import { escape as escapeRegExp, getFilepath, DELAY } from './utils.ts'; @@ - new RegExp(escape(`reload worker because ${filepath}`)) + new RegExp(escapeRegExp(`reload worker because ${filepath}`)) @@ - new RegExp(escape(`reload worker because ${filepath}`)) + new RegExp(escapeRegExp(`reload worker because ${filepath}`))Also applies to: 31-33, 43-45
48-64: Title vs expectation: make deterministic and consistent.Title says “once” but assertion allows “>= 1”. Either assert exactly one reload within the debounce window, or retitle to “at least once.” Also use .ts for consistency with the TS fixture.
-it('should reload once when 2 file change', async () => { +it('should reload once when two service files change within the debounce window', async () => { @@ - const filepath = getFilepath('development-ts/app/service/c.js'); - const filepath1 = getFilepath('development-ts/app/service/d.js'); + const filepath = getFilepath('development-ts/app/service/c.ts'); + const filepath1 = getFilepath('development-ts/app/service/d.ts'); @@ - assert(count(app.stdout, 'reload worker') >= 1); + assert.equal(count(app.stdout, 'reload worker'), 1);
20-22: Single async teardown: await app.close and debounce once.Merge the two afterEach hooks so the runner waits for clean shutdown.
- afterEach(() => app.close()); - // for debounce - afterEach(() => scheduler.wait(500)); + afterEach(async () => { + await app.close(); + await scheduler.wait(DELAY / 10); + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/cluster/test/app_worker.test.ts(2 hunks)packages/egg/package.json(1 hunks)plugins/development/src/agent.ts(1 hunks)plugins/development/test/absolute.test.ts(1 hunks)plugins/development/test/custom.test.ts(1 hunks)plugins/development/test/development-ts.test.ts(1 hunks)plugins/development/test/not-reload.test.ts(1 hunks)plugins/development/test/override.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/egg/package.json
- plugins/development/test/absolute.test.ts
- plugins/development/src/agent.ts
- plugins/development/test/custom.test.ts
🧰 Additional context used
📓 Path-based instructions (2)
packages/**/test/**/*.test.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/**/test/**/*.test.ts: Name test files as test/**/*.test.ts and run them with Vitest
Use import { describe, it } from 'vitest' in tests
Use Node.js built-in assert module for test assertions
packages/**/test/**/*.test.ts: All new packages must use Vitest as the test runner
Vitest tests should import from vitest (e.g., import { describe, it } from 'vitest')
Files:
packages/cluster/test/app_worker.test.ts
{packages,tools}/**/test/**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
{packages,tools}/**/test/**/*.test.ts: Test files should follow the naming pattern test/**/*.test.ts
Use Node.js built-in assert for assertions in tests
Files:
packages/cluster/test/app_worker.test.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to packages/egg/src/config/{plugin.ts,config.default.ts} : Maintain built-in plugin and default configuration in packages/egg/src/config/plugin.ts and config.default.ts
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to tools/egg-bin/test/**/*.test.ts : egg-bin uses Mocha for tests (exception to Vitest rule)
Applied to files:
plugins/development/test/not-reload.test.tsplugins/development/test/development-ts.test.tsplugins/development/test/override.test.ts
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to packages/egg/src/config/{plugin.ts,config.default.ts} : Maintain built-in plugin and default configuration in packages/egg/src/config/plugin.ts and config.default.ts
Applied to files:
plugins/development/test/development-ts.test.tsplugins/development/test/override.test.ts
📚 Learning: 2025-09-14T08:41:30.606Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.606Z
Learning: Applies to packages/**/test/**/*.test.ts : Name test files as test/**/*.test.ts and run them with Vitest
Applied to files:
plugins/development/test/development-ts.test.tsplugins/development/test/override.test.ts
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to packages/**/test/**/*.test.ts : Vitest tests should import from vitest (e.g., import { describe, it } from 'vitest')
Applied to files:
plugins/development/test/development-ts.test.tsplugins/development/test/override.test.ts
📚 Learning: 2025-09-14T08:41:30.606Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.606Z
Learning: Applies to packages/**/test/**/*.test.ts : Use import { describe, it } from 'vitest' in tests
Applied to files:
plugins/development/test/development-ts.test.tsplugins/development/test/override.test.ts
📚 Learning: 2025-09-14T08:41:30.606Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.606Z
Learning: Applies to packages/**/test/**/*.test.ts : Use Node.js built-in assert module for test assertions
Applied to files:
plugins/development/test/development-ts.test.ts
🧬 Code graph analysis (4)
plugins/development/test/not-reload.test.ts (1)
plugins/development/test/utils.ts (3)
getFilepath(7-9)DELAY(15-15)escape(11-13)
packages/cluster/test/app_worker.test.ts (1)
packages/egg/test/utils.ts (1)
app(40-45)
plugins/development/test/development-ts.test.ts (1)
plugins/development/test/utils.ts (3)
getFilepath(7-9)escape(11-13)DELAY(15-15)
plugins/development/test/override.test.ts (1)
plugins/development/test/utils.ts (2)
getFilepath(7-9)escape(11-13)
🪛 Biome (2.1.2)
plugins/development/test/not-reload.test.ts
[error] 6-6: Do not shadow the global "escape" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
plugins/development/test/development-ts.test.ts
[error] 8-8: Do not shadow the global "escape" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
plugins/development/test/override.test.ts
[error] 6-6: Do not shadow the global "escape" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🪛 ast-grep (0.38.6)
plugins/development/test/development-ts.test.ts
[warning] 67-67: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(match, 'g')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Test egg-bin (windows-latest, 24)
- GitHub Check: Test egg-bin (windows-latest, 22)
- GitHub Check: Test egg-bin (ubuntu-latest, 24)
- GitHub Check: Test packages (windows-latest, 24)
- GitHub Check: Test packages (windows-latest, 22)
- GitHub Check: Test packages (ubuntu-latest, 22)
- GitHub Check: Test egg-bin (ubuntu-latest, 22)
- GitHub Check: Test packages (ubuntu-latest, 24)
🔇 Additional comments (1)
plugins/development/test/not-reload.test.ts (1)
1-35: AI summary mismatch: mocks are not restored.Summary claims “restores mocks after each test,” but the code doesn’t. The added
afterEach(mm.restore())above resolves this.
| await scheduler.wait(10000); | ||
|
|
||
| app.expect('stdout', /app_worker#3:\d+ started at \d+/); | ||
| await app.close(); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Avoid double-closing the app; rely on global afterEach or nullify app after manual close
You already close the app in a file-level afterEach(() => app && app.close()). This extra await app.close() risks double-close and test flakiness.
Apply this minimal fix:
- await app.close();Alternative: keep early teardown but set app = undefined and change the declaration to let app: MockApplication | undefined; and afterEach(async () => { if (app) { await app.close(); app = undefined; } });.
Run to check for other manual closes in this file:
🏁 Script executed:
#!/bin/bash
rg -n 'await\s+app\.close\(' packages/cluster/test/app_worker.test.ts
rg -n 'afterEach\([^)]*app\s*&&\s*app\.close' packages/cluster/test/app_worker.test.tsLength of output: 202
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='packages/cluster/test/app_worker.test.ts'
echo "FILE: $file"
echo "---- afterEach occurrences ----"
rg -n --hidden 'afterEach' "$file" || true
echo
echo "---- app declaration / initialization (let/const/var) ----"
rg -n --hidden '\blet\s+app\b|\bconst\s+app\b|\bvar\s+app\b' "$file" || true
echo
echo "---- await app.close occurrences with context ----"
rg -n -C2 'await\s+app\.close\(' "$file" || true
echo
echo "---- any app?.close / app && app.close patterns with context ----"
rg -nP -C2 'app\?\.\s*close|app\s*&&\s*app\.close' "$file" || trueLength of output: 1392
Avoid double-closing the app — remove per-test manual closes or nullify app after manual close
There is a file-level afterEach(() => app && app.close()) at line 26 and manual await app.close() calls at lines 114, 132, 155, 184, 226, 364, 376. Remove the per-test/inline await app.close() calls (minimal fix):
-
await app.close();
Or, if an early teardown is required, change the declaration to let app: MockApplication | undefined; and use an async afterEach that awaits and nulls the app:
afterEach(async () => { if (app) { await app.close(); app = undefined; } });
🤖 Prompt for AI Agents
In packages/cluster/test/app_worker.test.ts around line 364, the test performs a
manual await app.close() while there is a file-level afterEach that also closes
the app, causing double-close; either remove the per-test inline await
app.close() at line 364 (and the other manual closes at 114,132,155,184,226,376)
so the global afterEach handles teardown, or change the app declaration to be
nullable (let app: MockApplication | undefined) and replace the afterEach with
an async one that awaits the close and nulls app (afterEach(async () => { if
(app) { await app.close(); app = undefined; } })) to allow early per-test
closure without double-closing.
| app.expect('stderr', /app_worker#1:\d+ start fail/); | ||
| app.expect('code', 1); | ||
|
|
||
| await app.close(); |
There was a problem hiding this comment.
Same here: duplicate teardown
This await app.close() duplicates the global afterEach close and can cause intermittent failures.
Apply this fix:
- await app.close();📝 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.
| await app.close(); |
🤖 Prompt for AI Agents
In packages/cluster/test/app_worker.test.ts around line 376, there is a
duplicated teardown call `await app.close()` that conflicts with the global
afterEach close and can cause intermittent failures; remove this redundant
`await app.close()` (or alternatively guard it behind a check so it only runs if
the global teardown won't) so the test relies on the shared afterEach cleanup
and avoids double-closing the app.
| function count(str: string, match: string) { | ||
| const m = str.match(new RegExp(match, 'g')); | ||
| return m ? m.length : 0; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid regex-from-variable in count(); use plain substring count.
Removes ReDoS risk and unnecessary regex.
function count(str: string, match: string) {
- const m = str.match(new RegExp(match, 'g'));
- return m ? m.length : 0;
+ if (!match) return 0;
+ return str.split(match).length - 1;
}📝 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.
| function count(str: string, match: string) { | |
| const m = str.match(new RegExp(match, 'g')); | |
| return m ? m.length : 0; | |
| } | |
| function count(str: string, match: string) { | |
| if (!match) return 0; | |
| return str.split(match).length - 1; | |
| } |
🧰 Tools
🪛 ast-grep (0.38.6)
[warning] 67-67: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(match, 'g')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🤖 Prompt for AI Agents
In plugins/development/test/development-ts.test.ts around lines 67 to 70, the
count() helper currently builds a RegExp from a variable which introduces ReDoS
risk and is unnecessary; replace it with a plain substring count implementation
(e.g., loop using indexOf and advance by match.length until no more occurrences)
so you avoid dynamic regex creation and correctly return the number of
non-overlapping occurrences; ensure it handles empty match safely (return 0) and
preserves the function signature and return semantics.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (9)
plugins/development/test/development.test.ts (1)
49-52: Use Vitest’s conditional runner instead of manual CI return.Clearer and avoids reporting a false “pass” when the body early-returns. Also fix wording.
-it('should reload once when 2 file change', async () => { - if (process.env.CI) { - return; - } +it.runIf(!process.env.CI)('should reload once when two files change', async () => {plugins/development/test/development-ts.test.ts (6)
50-53: Avoid skipping on CI; aim for event-driven readiness instead of timers.
Early-return on CI cuts coverage; once DELAY usage lands, consider re-enabling and/or waiting on a reload log/event instead of time.
50-50: Align test title with assertion.
Title says “reload once” but assertion is “>= 1”. Clarify intent.-it('should reload once when 2 file change', async () => { +it('should reload at least once when two file changes occur', async () => {
22-22: Tie debounce wait to DELAY to reduce flakiness.
Use the shared timing constant instead of a magic number.- afterEach(() => scheduler.wait(500)); + afterEach(() => scheduler.wait(DELAY / 10));
28-31: Replace hard-coded waits with DELAY-based values.
Makes tests stable across CI/local.- await scheduler.wait(1000); + await scheduler.wait(DELAY / 5); @@ - await scheduler.wait(5000); + await scheduler.wait(DELAY);
41-47: Same: remove magic waits in second test.- await scheduler.wait(1000); + await scheduler.wait(DELAY / 5); @@ - await scheduler.wait(5000); + await scheduler.wait(DELAY);
71-74: Remove dynamic RegExp in count(); use substring count.
Safer and simpler; avoids ReDoS lint and unnecessary regex.function count(str: string, match: string) { - const m = str.match(new RegExp(match, 'g')); - return m ? m.length : 0; + if (!match) return 0; + return str.split(match).length - 1; }plugins/development/test/not-reload.test.ts (2)
5-5: Restore mocks between tests to avoid env leakage.-import { beforeAll, afterAll, it, describe } from 'vitest'; +import { beforeAll, afterAll, afterEach, it, describe } from 'vitest'; @@ - afterAll(() => app.close()); + afterAll(() => app.close()); + afterEach(() => mm.restore());Also applies to: 23-23
6-6: Fix lint error: rename escape -> escapeRegExp and update usage.Biome flags shadowing of global escape; rename to escapeRegExp and adjust RegExp construction.
-import { escape, getFilepath, DELAY } from './utils.ts'; +import { escapeRegExp, getFilepath, DELAY } from './utils.ts'; @@ - new RegExp(escape(`reload worker because ${filepath} change`)) + new RegExp(escapeRegExp(`reload worker because ${filepath} change`))Apply in utils as well:
-// plugins/development/test/utils.ts -export function escape(str: string) { +// plugins/development/test/utils.ts +export function escapeRegExp(str: string) { return str.replace(/[|\\{}()[\]^$+*?.]/g, '\\$&').replace(/-/g, '\\x2d'); } +// temporary alias to ease migration; remove once all call sites are updated +// export { escapeRegExp as escape };Also applies to: 34-35
🧹 Nitpick comments (5)
plugins/development/test/fixtures/development-ts/config/plugin.ts (1)
1-3: Optional: add EggPlugin typing to catch key/name typos at compile timeThis improves type-safety without changing behavior.
Apply within this range:
-export default { - development: true, -}; +export default { + development: true, +} as EggPlugin;Add the import at the top of the file:
import type { EggPlugin } from 'egg';plugins/development/test/development.test.ts (4)
70-73: Avoid variable-driven RegExp incount(test-only, but easy to harden).Switch to a split-based count to remove ReDoS risk and quiet static analysis.
-function count(str: string, match: string) { - const m = str.match(new RegExp(match, 'g')); - return m ? m.length : 0; -} +function count(haystack: string, needle: string) { + if (!needle) return 0; + return haystack.split(needle).length - 1; +}
19-19: Ensure cleanup and restore mocks after tests.Close asynchronously and restore
mmto avoid env bleed between suites.-afterAll(() => app.close()); +afterAll(async () => { + await app.close(); + mm.restore(); +});
23-34: Wrap file ops in try/finally to guarantee cleanup on failure.If an assertion throws, the temp file may linger and affect subsequent runs.
I can provide a refactor wrapping this test body in
try { ... } finally { await fs.rm(filepath, { force: true }) }for deterministic cleanup—say the word and I’ll post a patch.
54-55: Minor naming nit: clarify second path variable.
filepath1reads as an index;filepath2orsecondFilepathis clearer.-const filepath1 = getFilepath('development/app/service/d.js'); +const filepath2 = getFilepath('development/app/service/d.js');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
plugins/development/test/development-ts.test.ts(1 hunks)plugins/development/test/development.test.ts(1 hunks)plugins/development/test/fixtures/absolute/config/plugin.js(1 hunks)plugins/development/test/fixtures/custom/config/plugin.js(1 hunks)plugins/development/test/fixtures/delay-ready/config/plugin.js(1 hunks)plugins/development/test/fixtures/development-process_mode_single/config/plugin.js(1 hunks)plugins/development/test/fixtures/development-ts/config/plugin.ts(1 hunks)plugins/development/test/fixtures/development/config/plugin.js(1 hunks)plugins/development/test/fixtures/fast-ready/config/plugin.js(1 hunks)plugins/development/test/fixtures/not-reload/app/service/a.js(1 hunks)plugins/development/test/fixtures/not-reload/config/plugin.js(1 hunks)plugins/development/test/fixtures/override-ignore/config/plugin.js(1 hunks)plugins/development/test/fixtures/override/config/plugin.js(1 hunks)plugins/development/test/fixtures/timing/config/plugin.js(1 hunks)plugins/development/test/not-reload.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- plugins/development/test/fixtures/development/config/plugin.js
- plugins/development/test/fixtures/not-reload/app/service/a.js
- plugins/development/test/fixtures/fast-ready/config/plugin.js
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to packages/egg/src/config/{plugin.ts,config.default.ts} : Maintain built-in plugin and default configuration in packages/egg/src/config/plugin.ts and config.default.ts
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to packages/egg/src/config/{plugin.ts,config.default.ts} : Maintain built-in plugin and default configuration in packages/egg/src/config/plugin.ts and config.default.ts
Applied to files:
plugins/development/test/fixtures/development-ts/config/plugin.tsplugins/development/test/development-ts.test.ts
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to tools/egg-bin/test/**/*.test.ts : egg-bin uses Mocha for tests (exception to Vitest rule)
Applied to files:
plugins/development/test/development-ts.test.tsplugins/development/test/development.test.tsplugins/development/test/not-reload.test.ts
📚 Learning: 2025-09-14T08:41:30.606Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.606Z
Learning: Applies to packages/**/test/**/*.test.ts : Name test files as test/**/*.test.ts and run them with Vitest
Applied to files:
plugins/development/test/development-ts.test.ts
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to packages/**/test/**/*.test.ts : Vitest tests should import from vitest (e.g., import { describe, it } from 'vitest')
Applied to files:
plugins/development/test/development-ts.test.tsplugins/development/test/development.test.tsplugins/development/test/not-reload.test.ts
📚 Learning: 2025-09-14T08:41:30.606Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.606Z
Learning: Applies to packages/**/test/**/*.test.ts : Use import { describe, it } from 'vitest' in tests
Applied to files:
plugins/development/test/development-ts.test.tsplugins/development/test/not-reload.test.ts
📚 Learning: 2025-09-14T08:41:30.606Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.606Z
Learning: Applies to packages/**/test/**/*.test.ts : Use Node.js built-in assert module for test assertions
Applied to files:
plugins/development/test/development-ts.test.ts
🧬 Code graph analysis (3)
plugins/development/test/development-ts.test.ts (1)
plugins/development/test/utils.ts (3)
getFilepath(7-9)escape(11-13)DELAY(15-15)
plugins/development/test/development.test.ts (1)
plugins/development/test/utils.ts (3)
getFilepath(7-9)escape(11-13)DELAY(15-15)
plugins/development/test/not-reload.test.ts (1)
plugins/development/test/utils.ts (3)
getFilepath(7-9)DELAY(15-15)escape(11-13)
🪛 ast-grep (0.38.6)
plugins/development/test/development-ts.test.ts
[warning] 71-71: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(match, 'g')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
plugins/development/test/development.test.ts
[warning] 70-70: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(match, 'g')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🪛 Biome (2.1.2)
plugins/development/test/development-ts.test.ts
[error] 8-8: Do not shadow the global "escape" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
plugins/development/test/development.test.ts
[error] 8-8: Do not shadow the global "escape" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
plugins/development/test/not-reload.test.ts
[error] 6-6: Do not shadow the global "escape" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: Test packages (windows-latest, 24)
- GitHub Check: Test packages (windows-latest, 22)
🔇 Additional comments (12)
plugins/development/test/fixtures/development-ts/config/plugin.ts (2)
1-3: LGTM: TS fixture correctly enables the development pluginLooks good as a minimal TS fixture.
1-3: Ensure the test runner loads .ts fixturesFindings: plugins/development/tsdown.config.ts and plugins/development/tsconfig.json exist, and plugins/development/package.json defines "build": "tsdown" and "test": "vitest run". No ts-node/ts-register or test bootstrap was found under plugins/development.
Action: either run the tsdown build (pnpm run build) before tests so fixtures are JS, or configure Vitest to transform .ts fixtures at runtime (enable a TS transformer such as esbuild/swc/ts-node in the vitest config).plugins/development/test/development.test.ts (1)
5-7: Vitest usage LGTM.Using Vitest here aligns with repo conventions (Mocha is only for egg-bin tests).
plugins/development/test/development-ts.test.ts (1)
5-7: Good: follows repo test conventions (Vitest + Node assert).
Imports align with learnings for packages//test//*.test.ts.plugins/development/test/fixtures/absolute/config/plugin.js (1)
1-1: LGTM — minimal, correct enable for this fixture.plugins/development/test/fixtures/override-ignore/config/plugin.js (1)
1-1: LGTM — correctly enables the plugin for the override-ignore fixture.plugins/development/test/fixtures/timing/config/plugin.js (1)
1-1: LGTM — correct enablement for timing fixture.plugins/development/test/fixtures/delay-ready/config/plugin.js (1)
1-1: LGTM — correct enablement for delay-ready fixture.plugins/development/test/fixtures/custom/config/plugin.js (1)
1-1: LGTM — aligns with custom fixture’s config; no issues.plugins/development/test/fixtures/development-process_mode_single/config/plugin.js (1)
1-1: LGTM — correct enablement for process_mode_single fixture.plugins/development/test/fixtures/not-reload/config/plugin.js (1)
1-1: LGTM — correct enablement for not-reload fixture.plugins/development/test/fixtures/override/config/plugin.js (1)
1-1: LGTM — fixture enables the development plugin (verified)All fixtures in plugins/development/test/fixtures set exports.development = true; core config (packages/egg/src/config/plugin.ts) defaults development.enable to process.env.CI ? false : true.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
plugins/development/test/custom.test.ts(1 hunks)pnpm-workspace.yaml(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/development/test/custom.test.ts
🧰 Additional context used
📓 Path-based instructions (1)
pnpm-workspace.yaml
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Define dependency versions via the catalog section in pnpm-workspace.yaml and reference them with "catalog:"
Define and manage centralized dependency versions in the catalog section
Files:
pnpm-workspace.yaml
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to packages/egg/src/config/{plugin.ts,config.default.ts} : Maintain built-in plugin and default configuration in packages/egg/src/config/plugin.ts and config.default.ts
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to {packages,tools}/**/package.json : Use catalog: specifiers for external dependencies defined in pnpm-workspace.yaml catalog
Applied to files:
pnpm-workspace.yaml
📚 Learning: 2025-09-14T08:41:30.606Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.606Z
Learning: Applies to pnpm-workspace.yaml : Define dependency versions via the catalog section in pnpm-workspace.yaml and reference them with "catalog:"
Applied to files:
pnpm-workspace.yaml
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to pnpm-workspace.yaml : Define and manage centralized dependency versions in the catalog section
Applied to files:
pnpm-workspace.yaml
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to {packages,tools}/**/package.json : Use workspace:* for internal workspace dependencies
Applied to files:
pnpm-workspace.yaml
📚 Learning: 2025-09-14T08:41:30.606Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.606Z
Learning: Applies to packages/*/package.json : Use "workspace:*" for internal workspace dependencies in package.json
Applied to files:
pnpm-workspace.yaml
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to tools/egg-bin/package.json : tools/egg-bin should be built on oclif/core
Applied to files:
pnpm-workspace.yaml
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to packages/egg/src/config/{plugin.ts,config.default.ts} : Maintain built-in plugin and default configuration in packages/egg/src/config/plugin.ts and config.default.ts
Applied to files:
pnpm-workspace.yaml
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to packages/egg/package.json : packages/egg should expose hybrid CommonJS/ESM exports
Applied to files:
pnpm-workspace.yaml
📚 Learning: 2025-09-14T08:42:15.407Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T08:42:15.407Z
Learning: Applies to packages/**/test/**/*.test.ts : Vitest tests should import from vitest (e.g., import { describe, it } from 'vitest')
Applied to files:
pnpm-workspace.yaml
📚 Learning: 2025-09-14T08:41:30.606Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.606Z
Learning: Require Node.js version >= 20.19.0 for all development and CI environments
Applied to files:
pnpm-workspace.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Test egg-bin (windows-latest, 22)
- GitHub Check: Test egg-bin (ubuntu-latest, 24)
- GitHub Check: Test egg-bin (ubuntu-latest, 22)
- GitHub Check: Test packages (macos-latest, 24)
- GitHub Check: Test egg-bin (windows-latest, 24)
- GitHub Check: Test packages (macos-latest, 22)
- GitHub Check: Test packages (windows-latest, 24)
- GitHub Check: Test packages (windows-latest, 22)
🔇 Additional comments (3)
pnpm-workspace.yaml (3)
3-3: Add workspace scope for plugins — approvedVerified: pnpm-workspace.yaml contains "plugins/" and vitest.config.ts 'projects' includes "plugins/".
140-142: Remove tshy/tshy-after from pnpm-workspace.yaml if unused — verify before deleting.PR migrated to tsdown but pnpm-workspace.yaml still lists tshy/tshy-after; remove if unused.
File: pnpm-workspace.yaml (lines 140–142)
Apply this diff if unused:
tsdown: ^0.15.0 - tshy: ^3.0.0 - tshy-after: ^1.0.0Check for lingering references:
#!/bin/bash rg -n -S --hidden --no-ignore -C2 '\b(tshy|tshy-after)\b' --glob '!**/node_modules/**'
54-55: Confirm Vitest 4 beta upgrade and run full repo tests.pnpm-workspace.yaml pins @vitest/coverage-v8 and @vitest/ui to 4.0.0-beta.11 (lines 54–55). package.json entries use catalog: for vitest at ./package.json, ./packages/extend2/package.json, ./packages/tsconfig/package.json — no explicit v3 pins found. Repo has many vitest.config.ts files and widespread imports from 'vitest'; quick grep found no occurrences of vi.mocked, mockModule, vi.resetModules, or vitestPreview.
Run the full test matrix/CI and verify @vitest/ui and @vitest/coverage-v8 runtime behavior; if tests fail, revert or pin to a stable vitest and update configs.
1 participant
Merge @eggjs/development plugin from its separate repository into the monorepo under plugins/development directory. This establishes the plugins directory structure for all Egg framework plugins.
BREAKING CHANGE: @eggjs/development is now part of the monorepo
🤖 Generated with Claude Code
Summary by CodeRabbit