Conversation
- Merge tool from https://github.com/eggjs/scripts - Update version to 5.0.0-beta.26 - Migrate to monorepo structure following CLAUDE.md standards - Add catalog dependencies (common-bin, mz, mz-modules, source-map-support) - Migrate tests from Mocha to Vitest - Create standard tsdown.config.ts with unplugin-unused - Update imports to use .ts extensions - Fix TypeScript type imports and unused variable warnings - Add tool reference to root tsconfig.json - Keep CHANGELOG.md for version history 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. 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 tools/scripts package (egg-scripts/eggctl) with CLI commands (start/stop), helpers, shims, docs, license, many tests and fixtures; updates workspace deps, CI shard matrix, TypeScript/vitest references, and adjusts assorted package scripts and test configs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as eggctl (oclif)
participant StartCmd as Start Command
participant Shim as start-cluster.(mjs|cjs)
participant Framework as Framework Module
participant Cluster as startCluster()
participant Worker as App Worker
User->>CLI: eggctl start [BASEDIR] [--flags]
CLI->>StartCmd: execute()
StartCmd->>StartCmd: parse flags & package config
StartCmd->>Shim: spawn Node with execArgv + JSON options
note right of Shim: daemon or foreground modes
Shim->>Framework: importModule(options.framework)
Framework-->>Shim: { startCluster }
Shim->>Cluster: call startCluster(options)
Cluster->>Worker: fork workers
Worker-->>Shim: IPC "egg-ready"
Shim-->>StartCmd: forward ready / exit
StartCmd-->>User: print status, log paths
sequenceDiagram
autonumber
actor User
participant CLI as eggctl (oclif)
participant StopCmd as Stop Command
participant Finder as findNodeProcess
participant OS as OS (ps / WMIC & signals)
User->>CLI: eggctl stop [BASEDIR] [--title] [--timeout]
CLI->>StopCmd: execute()
StopCmd->>Finder: scan node processes
Finder-->>StopCmd: masters & workers list
alt masters found
StopCmd->>OS: send SIGTERM to masters
StopCmd->>OS: send SIGKILL/SIGTERM to workers as needed
else none found
StopCmd-->>User: "No running Egg processes"
end
StopCmd-->>User: "stopped"
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
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: |
2eff4d9
|
| Status: | 🚫 Build failed. |
There was a problem hiding this comment.
Pull Request Overview
This PR merges the @eggjs/scripts tool from a separate repository into the main Eggjs monorepo structure. The main purpose is to consolidate the Eggjs tooling ecosystem and modernize the scripts package with updated dependencies and TypeScript support.
Key changes:
- Migrate @eggjs/scripts from external repository to tools/scripts/ directory
- Upgrade to version 5.0.0-beta.26 with monorepo catalog dependencies
- Convert tests from Mocha to Vitest framework
Reviewed Changes
Copilot reviewed 82 out of 89 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tools/scripts/ | Complete new package directory with source code, tests, and configuration |
| pnpm-workspace.yaml | Add catalog dependencies for common-bin, mz, mz-modules, and source-map-support |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
tools/scripts/test/start.test.ts
Outdated
| await cleanup(fixturePath); | ||
| }); | ||
|
|
||
| beforeEach(async () => { |
There was a problem hiding this comment.
Duplicate beforeEach hooks performing similar cleanup operations should be consolidated into a single hook to avoid redundant cleanup calls.
| await cleanup(fixturePath); | |
| }); | |
| beforeEach(async () => { |
tools/scripts/test/stop.test.ts
Outdated
| describe('stop all with timeout', function () { | ||
| let app: Coffee; | ||
| let killer: Coffee; | ||
| this.timeout(17000); |
There was a problem hiding this comment.
Using function declaration and this.timeout() is Mocha syntax but the test imports Vitest. This will cause runtime errors since Vitest doesn't support this.timeout().
Deploying egg-v3 with
|
| Latest commit: |
2eff4d9
|
| Status: | ⚡️ Build in progress... |
Summary of ChangesHello @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 integrates the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully merges the @eggjs/scripts tool into the monorepo, updating it to a modern TypeScript and oclif-based structure. The migration to Vitest and pnpm workspaces is well-executed. My review focuses on improving cross-platform compatibility, code clarity, and correctness. I've identified a few areas for improvement, including an issue with non-portable shell commands, a potential bug in process matching on Windows, and an opportunity to refactor a utility function for better readability.
| const debug = debuglog('@eggjs/scripts/commands/stop'); | ||
|
|
||
| const osRelated = { | ||
| titleTemplate: isWindows ? '\\"title\\":\\"%s\\"' : '"title":"%s"', |
There was a problem hiding this comment.
The escaping for titleTemplate on Windows seems incorrect. It will produce a string with literal backslashes, like \"title\":\"the-title\", which is unlikely to match the process command line. It should probably be the same as the non-Windows version to correctly find and stop the process.
| titleTemplate: isWindows ? '\\"title\\":\\"%s\\"' : '"title":"%s"', | |
| titleTemplate: '"title":"%s"', |
| try { | ||
| const args = ['-n', '100', stderrFile]; | ||
| this.logToStderr('tail %s', args.join(' ')); | ||
| const { stdout: headStdout } = await execFile('head', args); | ||
| const { stdout: tailStdout } = await execFile('tail', args); | ||
| this.logToStderr('Got error when startup: '); | ||
| this.logToStderr(headStdout); | ||
| this.logToStderr('...'); | ||
| this.logToStderr(tailStdout); | ||
| } catch (err) { | ||
| this.logToStderr('ignore tail error: %s', err); | ||
| } |
There was a problem hiding this comment.
The use of head and tail commands is not portable and will fail on Windows. This can be replaced with a cross-platform solution using Node.js fs.readFile. Since this happens at startup, the log file is not expected to be very large, so reading the whole file should be acceptable.
You'll also need to add readFile to the node:fs/promises import on line 5.
| try { | |
| const args = ['-n', '100', stderrFile]; | |
| this.logToStderr('tail %s', args.join(' ')); | |
| const { stdout: headStdout } = await execFile('head', args); | |
| const { stdout: tailStdout } = await execFile('tail', args); | |
| this.logToStderr('Got error when startup: '); | |
| this.logToStderr(headStdout); | |
| this.logToStderr('...'); | |
| this.logToStderr(tailStdout); | |
| } catch (err) { | |
| this.logToStderr('ignore tail error: %s', err); | |
| } | |
| try { | |
| const stderrContent = await readFile(stderrFile, 'utf-8'); | |
| const lines = stderrContent.split('\n'); | |
| this.logToStderr('Got error when startup:'); | |
| if (lines.length <= 200) { | |
| this.logToStderr(stderrContent); | |
| } else { | |
| this.logToStderr(lines.slice(0, 100).join('\n')); | |
| this.logToStderr('\n...\n'); | |
| this.logToStderr(lines.slice(-100).join('\n')); | |
| } | |
| } catch (err) { | |
| this.logToStderr('ignore read stderr file error: %s', err); | |
| } |
| function stringify(obj: Record<string, any>, ignore: string[]) { | ||
| const result: Record<string, any> = {}; | ||
| Object.keys(obj).forEach(key => { | ||
| if (!ignore.includes(key)) { | ||
| result[key] = obj[key]; | ||
| } | ||
| }); | ||
| return JSON.stringify(result); | ||
| } |
There was a problem hiding this comment.
This stringify function can be implemented more concisely and potentially more efficiently by creating a copy and deleting keys, rather than iterating and building a new object.
function stringify(obj: Record<string, any>, ignore: string[]) {
const result = { ...obj };
for (const key of ignore) {
delete result[key];
}
return JSON.stringify(result);
}| @@ -0,0 +1,6 @@ | |||
| import Start from './commands/start.js'; | |||
|
|
|||
| // exports.StopCommand = require('./lib/cmd/stop'); | |||
There was a problem hiding this comment.
This commented-out line appears to be a leftover from a previous implementation and can be removed for cleaner code.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (1)
tools/scripts/test/fixtures/ts/app/controller/home.ts (1)
1-15: Duplicate comment - see review of tools/scripts/test/fixtures/ts-pkg/app/controller/home.ts.
🧹 Nitpick comments (16)
tools/scripts/test/fixtures/trace-warnings/app.js (1)
12-13: Clarify deprecated Buffer usage in test fixture.The
new Buffer('aaa')call is intentionally used here to trigger a deprecation warning when testing--no-deprecation. Please expand the comment to state this purpose explicitly (e.g. “Trigger deprecation warning for Buffer; ensure--no-deprecationsuppresses only deprecation warnings, not other warning types”).tools/scripts/src/helper.ts (1)
52-60: Consider improving type safety for ESM interop.The
@ts-ignoredirective at lines 56-58 suppresses TypeScript checking forimport.meta.url. While this may be necessary for dual CJS/ESM support, consider:
- Ensuring
tsconfig.jsonhas propermoduleandmoduleResolutionsettings for ESM- Using a more specific
@ts-expect-errorwith a descriptive comment explaining why the ignore is needed- If possible, separating CJS and ESM entry points to avoid runtime checks
Example improvement:
- // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore + // @ts-expect-error - import.meta.url is only available in ESM context const __filename = fileURLToPath(import.meta.url);tools/scripts/test/fixtures/status/app.js (1)
8-10: Add validation for WAIT_TIME environment variable.The code uses
parseInt(process.env.WAIT_TIME)without validation. IfWAIT_TIMEis undefined or not a valid number,parseIntreturnsNaN, which could cause unexpected behavior when passed toscheduler.wait().Apply this diff to add a default value:
app.beforeStart(async () => { - await scheduler.wait(parseInt(process.env.WAIT_TIME)); + await scheduler.wait(parseInt(process.env.WAIT_TIME) || 0); });Alternatively, add explicit validation:
app.beforeStart(async () => { - await scheduler.wait(parseInt(process.env.WAIT_TIME)); + const waitTime = parseInt(process.env.WAIT_TIME); + if (isNaN(waitTime)) { + throw new Error('WAIT_TIME must be a valid number'); + } + await scheduler.wait(waitTime); });tools/scripts/test/fixtures/ts-pkg/package.json (1)
1-14: Consider adding Node.js engine requirement.Per coding guidelines, packages under
{packages,plugins,tools}/**/package.jsonshould require Node.js >= 22.18.0. While this is a test fixture, adding the engine requirement ensures consistency and prevents accidental usage with older Node versions.Consider applying this diff:
{ "name": "ts-pkg", "version": "1.0.0", + "engines": { + "node": ">=22.18.0" + }, "dependencies": {tools/scripts/src/index.ts (1)
3-3: Remove or implement the commented StopCommand export.The commented line suggests incomplete work or leftover code. Either implement and uncomment the StopCommand export, or remove this comment if the feature is not planned.
If you'd like me to help track this as a TODO or create an issue for implementing StopCommand, let me know.
tools/scripts/test/fixtures/ts-pkg/tsconfig.json (1)
1-26: Consider extending root tsconfig.json and setting baseUrl.Per coding guidelines, TypeScript configuration files under
{packages,plugins,tools}/**/tsconfig.jsonshould:
- Extend the root
tsconfig.json- Set
compilerOptions.baseUrlto"./"While this is a test fixture that may need independent configuration for testing purposes, consider whether it should follow the standard pattern for consistency.
As per coding guidelines.
If this fixture intentionally uses standalone configuration for testing isolation, consider documenting that decision with a comment in the file.
Apply this diff if standard configuration is appropriate:
{ + "extends": "../../../../../tsconfig.json", "compileOnSave": true, "compilerOptions": { + "baseUrl": "./", "target": "es2017",tools/scripts/test/fixtures/cluster-config/config/config.default.js (1)
1-6: LGTM! Standard configuration fixture.The exports follow the expected pattern for Egg.js configuration files.
Consider adding
'use strict';at the top for consistency with other fixtures (e.g.,stop-timeout/config/config.default.js), though it's optional for CommonJS modules.tools/scripts/test/fixtures/ts-pkg/app/controller/home.ts (2)
7-7: Consider usingunknowninstead ofany.The
anytype disables type checking. Since the error is immediately used only for logging and property access,unknownwith type narrowing is more type-safe.Apply this diff:
- } catch (err: any) { + } catch (err: unknown) { this.ctx.logger.error(err); this.ctx.body = { - msg: err.message, - stack: err.stack, + msg: err instanceof Error ? err.message : String(err), + stack: err instanceof Error ? err.stack : undefined, }; }
3-15: Duplicate code across test fixtures.This controller is duplicated in
tools/scripts/test/fixtures/ts/app/controller/home.ts(lines 3-15) andtools/scripts/test/fixtures/ts-pkg/app/controller/home.ts. Consider whether both fixtures are necessary or if they can share the same controller implementation.If both fixtures require identical controllers, consider extracting the common implementation to a shared location or using symbolic links to avoid maintaining duplicate code.
tools/scripts/test/fixtures/ipc-bin/start.js (1)
3-3: Consider modernizing from generators to async/await.The
colibrary and generator functions (function*,yield) are older patterns that have been superseded by native async/await since Node.js 8.For consistency with modern JavaScript and the rest of the codebase (which uses async/await), consider refactoring:
-const co = require('co'); - const BaseStartCommand = require('../../../lib/cmd/start'); class StartCommand extends BaseStartCommand { - *run(context) { - yield super.run(context); + async run(context) { + await super.run(context); const child = this.child; child.on('message', msg => { if (msg && msg.action === 'egg-ready') { console.log('READY!!!'); } }); } } const start = new StartCommand(); -co(function* () { - yield start.run({ +(async () => { + await start.run({ argv: { framework: 'custom-framework', _: [process.env.BASE_DIR], workers: 2, title: 'egg-server-example', }, cwd: process.env.BASE_DIR, - // FIXME: overide run argv so execArgvObj is missing + // FIXME: override run argv so execArgvObj is missing // execArgv: [], env: { PATH: process.env.PATH, }, }); -}); +})().catch(err => { + console.error('Start failed:', err); + process.exit(1); +});Note: Also fixed the typo "overide" → "override" in the FIXME comment.
tools/scripts/test/ts.test.ts (1)
116-116: Remove debugging console.log.The
console.log(result.data)statement should be removed or commented out to keep test output clean.Apply this diff:
const result = await request('http://127.0.0.1:7001', { dataType: 'json' }); - console.log(result.data); + // console.log(result.data); assert.match(result.data.stack, /home\.ts:6:13/);tools/scripts/src/baseCommand.ts (1)
18-23: Consider removing commented code.The commented-out
log-levelflag definition should either be uncommented if planned for near-term use or removed to reduce clutter. If it's a future enhancement, consider documenting it in a TODO or issue instead.If not needed soon, apply this diff:
// define flags that can be inherited by any command that extends BaseCommand static baseFlags = { - // 'log-level': Flags.option({ - // default: 'info', - // helpGroup: 'GLOBAL', - // options: ['debug', 'warn', 'error', 'info', 'trace'] as const, - // summary: 'Specify level for logging.', - // })(), };tools/scripts/src/commands/stop.ts (1)
59-69: Consider post-wait re-scan and escalation for stubborn mastersAfter waiting, re-scan and force-kill any surviving masters to avoid orphaned clusters. This keeps stop deterministic on noisy systems.
Apply this diff:
this.killProcesses(pids); // wait for 5s to confirm whether any worker process did not kill by master await scheduler.wait(flags.timeout); + // re-scan and escalate if masters still alive + const stillMasters = await this.findNodeProcesses(item => { + const cmd = item.cmd; + return flags.title + ? cmd.includes('start-cluster') && cmd.includes(format(osRelated.titleTemplate, flags.title)) + : cmd.includes('start-cluster'); + }); + const stillPids = stillMasters.map(x => x.pid); + if (stillPids.length) { + this.log('master still running after %dms, force killing %j', flags.timeout, stillPids); + // On Unix, SIGKILL ensures termination; on Windows this maps to TerminateProcess. + this.killProcesses(stillPids, 'SIGKILL'); + }If Windows CI is enabled, please verify SIGKILL behavior there. If it’s undesirable on Windows, we can conditionally fall back to SIGTERM.
tools/scripts/test/start.test.ts (2)
218-226: Reduce flakiness: wait for server to start listening before spawning the appEnsure the port is bound before starting Egg to deterministically trigger the “port in use” path.
Apply this diff:
- const server = createServer(() => {}); - server.listen(7007); + const server = createServer(() => {}); + server.listen(7007); + await once(server, 'listening');
521-529: Duplicate beforeEach hooks in the same describeTwo beforeEach blocks run in order; merge for clarity and to avoid surprises if future changes depend on ordering.
Combine both beforeEach bodies into one hook.
tools/scripts/src/commands/start.ts (1)
331-347: Cross‑platform stderr tailing and log message inconsistencieshead/tail are unavailable on Windows; you already catch errors, but we can avoid needless subprocesses on win32 and fix the misleading “tail …” log when running head first.
Apply this diff to guard shelling out by platform and clarify logging:
- if (hasError) { + if (hasError) { try { - const args = ['-n', '100', stderrFile]; - this.logToStderr('tail %s', args.join(' ')); - const { stdout: headStdout } = await execFile('head', args); - const { stdout: tailStdout } = await execFile('tail', args); - this.logToStderr('Got error when startup: '); - this.logToStderr(headStdout); - this.logToStderr('...'); - this.logToStderr(tailStdout); + if (process.platform !== 'win32') { + const args = ['-n', '100', stderrFile]; + this.logToStderr('head %s', args.join(' ')); + const { stdout: headStdout } = await execFile('head', args); + this.logToStderr('tail %s', args.join(' ')); + const { stdout: tailStdout } = await execFile('tail', args); + this.logToStderr('Got error when startup: '); + this.logToStderr(headStdout); + this.logToStderr('...'); + this.logToStderr(tailStdout); + } else { + this.logToStderr('Error output detected, see %o', stderrFile); + } } catch (err) { this.logToStderr('ignore tail error: %s', err); }Optionally, replace head/tail with a Node‑only reader (more portable). I can provide a small helper to read the last N lines.
📜 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 (88)
pnpm-workspace.yaml(3 hunks)tools/scripts/CHANGELOG.md(1 hunks)tools/scripts/LICENSE(1 hunks)tools/scripts/README.md(1 hunks)tools/scripts/bin/dev.cmd(1 hunks)tools/scripts/bin/dev.js(1 hunks)tools/scripts/bin/run.cmd(1 hunks)tools/scripts/bin/run.js(1 hunks)tools/scripts/package.json(1 hunks)tools/scripts/scripts/start-cluster.cjs(1 hunks)tools/scripts/scripts/start-cluster.mjs(1 hunks)tools/scripts/src/baseCommand.ts(1 hunks)tools/scripts/src/commands/start.ts(1 hunks)tools/scripts/src/commands/stop.ts(1 hunks)tools/scripts/src/helper.ts(1 hunks)tools/scripts/src/index.ts(1 hunks)tools/scripts/src/types.ts(1 hunks)tools/scripts/test/egg-scripts.test.ts(1 hunks)tools/scripts/test/fixtures/cluster-config/app/router.js(1 hunks)tools/scripts/test/fixtures/cluster-config/config/config.default.js(1 hunks)tools/scripts/test/fixtures/cluster-config/config/config.prod.js(1 hunks)tools/scripts/test/fixtures/cluster-config/package.json(1 hunks)tools/scripts/test/fixtures/custom-node-dir/.node/bin/foo(1 hunks)tools/scripts/test/fixtures/custom-node-dir/app/router.js(1 hunks)tools/scripts/test/fixtures/custom-node-dir/config/config.default.js(1 hunks)tools/scripts/test/fixtures/custom-node-dir/package.json(1 hunks)tools/scripts/test/fixtures/egg-app/config/config.default.js(1 hunks)tools/scripts/test/fixtures/egg-app/package.json(1 hunks)tools/scripts/test/fixtures/egg-revert/config/config.default.js(1 hunks)tools/scripts/test/fixtures/egg-revert/package.json(1 hunks)tools/scripts/test/fixtures/egg-scripts-config/app.js(1 hunks)tools/scripts/test/fixtures/egg-scripts-config/config/config.default.js(1 hunks)tools/scripts/test/fixtures/egg-scripts-config/package.json(1 hunks)tools/scripts/test/fixtures/egg-scripts-node-options/app.js(1 hunks)tools/scripts/test/fixtures/egg-scripts-node-options/config/config.default.js(1 hunks)tools/scripts/test/fixtures/egg-scripts-node-options/package.json(1 hunks)tools/scripts/test/fixtures/example/app.js(1 hunks)tools/scripts/test/fixtures/example/app/router.js(1 hunks)tools/scripts/test/fixtures/example/config/config.default.js(1 hunks)tools/scripts/test/fixtures/example/config/config.pre.js(1 hunks)tools/scripts/test/fixtures/example/package.json(1 hunks)tools/scripts/test/fixtures/ipc-bin/start.js(1 hunks)tools/scripts/test/fixtures/pkg-config-esm/config/config.default.js(1 hunks)tools/scripts/test/fixtures/pkg-config-esm/config/plugin.js(1 hunks)tools/scripts/test/fixtures/pkg-config-esm/inject1.js(1 hunks)tools/scripts/test/fixtures/pkg-config-esm/inject2.js(1 hunks)tools/scripts/test/fixtures/pkg-config-esm/package.json(1 hunks)tools/scripts/test/fixtures/pkg-config-sourcemap/config/config.default.js(1 hunks)tools/scripts/test/fixtures/pkg-config-sourcemap/config/plugin.js(1 hunks)tools/scripts/test/fixtures/pkg-config-sourcemap/package.json(1 hunks)tools/scripts/test/fixtures/pkg-config/config/config.default.js(1 hunks)tools/scripts/test/fixtures/pkg-config/config/plugin.js(1 hunks)tools/scripts/test/fixtures/pkg-config/inject1.js(1 hunks)tools/scripts/test/fixtures/pkg-config/inject2.js(1 hunks)tools/scripts/test/fixtures/pkg-config/package.json(1 hunks)tools/scripts/test/fixtures/status/app.js(1 hunks)tools/scripts/test/fixtures/status/config/config.default.js(1 hunks)tools/scripts/test/fixtures/status/package.json(1 hunks)tools/scripts/test/fixtures/stop-timeout/app.js(1 hunks)tools/scripts/test/fixtures/stop-timeout/app/router.js(1 hunks)tools/scripts/test/fixtures/stop-timeout/config/config.default.js(1 hunks)tools/scripts/test/fixtures/stop-timeout/package.json(1 hunks)tools/scripts/test/fixtures/subdir-as-basedir/base-dir/app/router.js(1 hunks)tools/scripts/test/fixtures/subdir-as-basedir/base-dir/config/config.default.js(1 hunks)tools/scripts/test/fixtures/subdir-as-basedir/base-dir/package.json(1 hunks)tools/scripts/test/fixtures/subdir-as-basedir/package.json(1 hunks)tools/scripts/test/fixtures/trace-warnings/app.js(1 hunks)tools/scripts/test/fixtures/trace-warnings/package.json(1 hunks)tools/scripts/test/fixtures/ts-pkg/app/controller/home.js(1 hunks)tools/scripts/test/fixtures/ts-pkg/app/controller/home.ts(1 hunks)tools/scripts/test/fixtures/ts-pkg/app/router.js(1 hunks)tools/scripts/test/fixtures/ts-pkg/config/config.default.js(1 hunks)tools/scripts/test/fixtures/ts-pkg/package.json(1 hunks)tools/scripts/test/fixtures/ts-pkg/tsconfig.json(1 hunks)tools/scripts/test/fixtures/ts/app/controller/home.js(1 hunks)tools/scripts/test/fixtures/ts/app/controller/home.ts(1 hunks)tools/scripts/test/fixtures/ts/app/router.js(1 hunks)tools/scripts/test/fixtures/ts/config/config.default.js(1 hunks)tools/scripts/test/fixtures/ts/package.json(1 hunks)tools/scripts/test/fixtures/ts/tsconfig.json(1 hunks)tools/scripts/test/start.test.ts(1 hunks)tools/scripts/test/stop.test.ts(1 hunks)tools/scripts/test/ts.test.ts(1 hunks)tools/scripts/test/utils.ts(1 hunks)tools/scripts/tsconfig.json(1 hunks)tools/scripts/tsdown.config.ts(1 hunks)tools/scripts/vitest.config.ts(1 hunks)tsconfig.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
**/test/fixtures/**
📄 CodeRabbit inference engine (AGENTS.md)
Put reusable test data under test/fixtures/
Files:
tools/scripts/test/fixtures/egg-scripts-node-options/config/config.default.jstools/scripts/test/fixtures/ts-pkg/app/router.jstools/scripts/test/fixtures/ts-pkg/config/config.default.jstools/scripts/test/fixtures/pkg-config-esm/inject2.jstools/scripts/test/fixtures/pkg-config/config/config.default.jstools/scripts/test/fixtures/pkg-config-sourcemap/config/config.default.jstools/scripts/test/fixtures/pkg-config/inject1.jstools/scripts/test/fixtures/stop-timeout/package.jsontools/scripts/test/fixtures/example/app.jstools/scripts/test/fixtures/cluster-config/app/router.jstools/scripts/test/fixtures/status/config/config.default.jstools/scripts/test/fixtures/stop-timeout/config/config.default.jstools/scripts/test/fixtures/ts/app/controller/home.tstools/scripts/test/fixtures/egg-app/config/config.default.jstools/scripts/test/fixtures/cluster-config/config/config.default.jstools/scripts/test/fixtures/egg-scripts-config/app.jstools/scripts/test/fixtures/example/package.jsontools/scripts/test/fixtures/trace-warnings/app.jstools/scripts/test/fixtures/pkg-config-esm/config/plugin.jstools/scripts/test/fixtures/pkg-config-esm/config/config.default.jstools/scripts/test/fixtures/egg-revert/config/config.default.jstools/scripts/test/fixtures/pkg-config-sourcemap/config/plugin.jstools/scripts/test/fixtures/egg-revert/package.jsontools/scripts/test/fixtures/custom-node-dir/app/router.jstools/scripts/test/fixtures/stop-timeout/app.jstools/scripts/test/fixtures/ts/app/controller/home.jstools/scripts/test/fixtures/ts/tsconfig.jsontools/scripts/test/fixtures/pkg-config/config/plugin.jstools/scripts/test/fixtures/subdir-as-basedir/base-dir/config/config.default.jstools/scripts/test/fixtures/egg-scripts-config/config/config.default.jstools/scripts/test/fixtures/cluster-config/config/config.prod.jstools/scripts/test/fixtures/egg-scripts-node-options/app.jstools/scripts/test/fixtures/ts-pkg/app/controller/home.tstools/scripts/test/fixtures/subdir-as-basedir/base-dir/app/router.jstools/scripts/test/fixtures/example/config/config.pre.jstools/scripts/test/fixtures/ts/app/router.jstools/scripts/test/fixtures/pkg-config/package.jsontools/scripts/test/fixtures/example/config/config.default.jstools/scripts/test/fixtures/ts/config/config.default.jstools/scripts/test/fixtures/stop-timeout/app/router.jstools/scripts/test/fixtures/custom-node-dir/package.jsontools/scripts/test/fixtures/custom-node-dir/config/config.default.jstools/scripts/test/fixtures/example/app/router.jstools/scripts/test/fixtures/status/package.jsontools/scripts/test/fixtures/subdir-as-basedir/package.jsontools/scripts/test/fixtures/ts-pkg/tsconfig.jsontools/scripts/test/fixtures/ts-pkg/package.jsontools/scripts/test/fixtures/cluster-config/package.jsontools/scripts/test/fixtures/pkg-config-esm/inject1.jstools/scripts/test/fixtures/pkg-config-sourcemap/package.jsontools/scripts/test/fixtures/trace-warnings/package.jsontools/scripts/test/fixtures/egg-app/package.jsontools/scripts/test/fixtures/ts-pkg/app/controller/home.jstools/scripts/test/fixtures/pkg-config-esm/package.jsontools/scripts/test/fixtures/ts/package.jsontools/scripts/test/fixtures/status/app.jstools/scripts/test/fixtures/subdir-as-basedir/base-dir/package.jsontools/scripts/test/fixtures/pkg-config/inject2.jstools/scripts/test/fixtures/egg-scripts-node-options/package.jsontools/scripts/test/fixtures/egg-scripts-config/package.jsontools/scripts/test/fixtures/ipc-bin/start.js
{package.json,{packages,plugins,tools}/**/package.json}
📄 CodeRabbit inference engine (CLAUDE.md)
{package.json,{packages,plugins,tools}/**/package.json}: All packages must require Node.js >= 22.18.0 (set engines.node accordingly)
Use workspace:* for internal dependencies and catalog: for external dependencies defined in pnpm-workspace.yaml
Files:
tools/scripts/test/fixtures/stop-timeout/package.jsontools/scripts/test/fixtures/example/package.jsontools/scripts/test/fixtures/egg-revert/package.jsontools/scripts/test/fixtures/pkg-config/package.jsontools/scripts/test/fixtures/custom-node-dir/package.jsontools/scripts/test/fixtures/status/package.jsontools/scripts/test/fixtures/subdir-as-basedir/package.jsontools/scripts/test/fixtures/ts-pkg/package.jsontools/scripts/test/fixtures/cluster-config/package.jsontools/scripts/test/fixtures/pkg-config-sourcemap/package.jsontools/scripts/test/fixtures/trace-warnings/package.jsontools/scripts/test/fixtures/egg-app/package.jsontools/scripts/test/fixtures/pkg-config-esm/package.jsontools/scripts/test/fixtures/ts/package.jsontools/scripts/test/fixtures/subdir-as-basedir/base-dir/package.jsontools/scripts/test/fixtures/egg-scripts-node-options/package.jsontools/scripts/test/fixtures/egg-scripts-config/package.jsontools/scripts/package.json
{packages,plugins,tools}/**/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
Each package must define scripts: typecheck as "tsc --noEmit" and lint as "oxlint --type-aware"
Files:
tools/scripts/test/fixtures/stop-timeout/package.jsontools/scripts/test/fixtures/example/package.jsontools/scripts/test/fixtures/egg-revert/package.jsontools/scripts/test/fixtures/pkg-config/package.jsontools/scripts/test/fixtures/custom-node-dir/package.jsontools/scripts/test/fixtures/status/package.jsontools/scripts/test/fixtures/subdir-as-basedir/package.jsontools/scripts/test/fixtures/ts-pkg/package.jsontools/scripts/test/fixtures/cluster-config/package.jsontools/scripts/test/fixtures/pkg-config-sourcemap/package.jsontools/scripts/test/fixtures/trace-warnings/package.jsontools/scripts/test/fixtures/egg-app/package.jsontools/scripts/test/fixtures/pkg-config-esm/package.jsontools/scripts/test/fixtures/ts/package.jsontools/scripts/test/fixtures/subdir-as-basedir/base-dir/package.jsontools/scripts/test/fixtures/egg-scripts-node-options/package.jsontools/scripts/test/fixtures/egg-scripts-config/package.jsontools/scripts/package.json
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Prefer TypeScript and ESM: write sources and exports in .ts (ESM-first) rather than CommonJS
Use two-space indentation, trailing commas, and semicolons (Prettier/oxlint defaults)
Name files in lowercase with hyphens (e.g., loader-context.ts)
Name classes in PascalCase
Name functions and variables in camelCase
Re-export types thoughtfully to keep the public API stable
Files:
tools/scripts/test/fixtures/ts/app/controller/home.tstools/scripts/src/index.tstools/scripts/src/types.tstools/scripts/src/helper.tstools/scripts/test/egg-scripts.test.tstools/scripts/test/fixtures/ts-pkg/app/controller/home.tstools/scripts/test/ts.test.tstools/scripts/src/commands/stop.tstools/scripts/src/baseCommand.tstools/scripts/vitest.config.tstools/scripts/test/start.test.tstools/scripts/test/stop.test.tstools/scripts/tsdown.config.tstools/scripts/src/commands/start.tstools/scripts/test/utils.ts
{packages,plugins,tools}/**/tsconfig.json
📄 CodeRabbit inference engine (CLAUDE.md)
{packages,plugins,tools}/**/tsconfig.json: All sub-project tsconfig.json files must extend the root tsconfig.json and set compilerOptions.baseUrl to "./"
Enable TypeScript strict mode in all TypeScript packages
Files:
tools/scripts/test/fixtures/ts/tsconfig.jsontools/scripts/tsconfig.jsontools/scripts/test/fixtures/ts-pkg/tsconfig.json
**/test/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/test/**/*.test.ts: Place test suites following Vitest discovery: /test//*.test.ts
Mirror the repository test pattern when adding new suitesTest files should be named with the pattern test/**/*.test.ts
Files:
tools/scripts/test/egg-scripts.test.tstools/scripts/test/ts.test.tstools/scripts/test/start.test.tstools/scripts/test/stop.test.ts
{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
{packages,plugins,tools}/**/vitest.config.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Each Vitest-using package should include a vitest.config.ts
Files:
tools/scripts/vitest.config.ts
{packages,plugins,tools}/**/tsdown.config.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Each package should use tsdown for builds and include a tsdown.config.ts
Files:
tools/scripts/tsdown.config.ts
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:"
Manage external dependency versions via the catalog in pnpm-workspace.yaml
Files:
pnpm-workspace.yaml
🧠 Learnings (24)
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to packages/egg/src/config/{plugin.ts,config.default.ts} : Define built-in plugin and default framework configuration within packages/egg/src/config
Applied to files:
tools/scripts/test/fixtures/pkg-config-esm/config/plugin.jstools/scripts/src/types.tstools/scripts/test/fixtures/pkg-config-sourcemap/config/plugin.jstools/scripts/test/fixtures/pkg-config/config/plugin.js
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to {packages,plugins,tools}/**/tsconfig.json : Enable TypeScript strict mode in all TypeScript packages
Applied to files:
tools/scripts/test/fixtures/ts/tsconfig.jsontools/scripts/test/fixtures/ts-pkg/tsconfig.json
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to {packages,plugins,tools}/**/tsconfig.json : All sub-project tsconfig.json files must extend the root tsconfig.json and set compilerOptions.baseUrl to "./"
Applied to files:
tools/scripts/tsconfig.json
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to packages/*/tsconfig.json : All sub-project tsconfig.json files must extend from the root via "extends": "../../tsconfig.json"
Applied to files:
tools/scripts/tsconfig.json
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to tools/egg-bin/test/**/*.test.ts : The egg-bin tool package must use Mocha for tests with files under test/**/*.test.ts
Applied to files:
tools/scripts/test/egg-scripts.test.tstools/scripts/test/ts.test.ts
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to packages/{egg,core,utils,mock,cluster,koa,supertest,extend2,koa-static-cache,router}/test/**/*.test.ts : Core packages must use Vitest (import from 'vitest') with test files under test/**/*.test.ts
Applied to files:
tools/scripts/test/egg-scripts.test.tstools/scripts/test/ts.test.ts
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to packages/**/test/**/*.test.ts : Use import { describe, it } from 'vitest' in tests
Applied to files:
tools/scripts/test/egg-scripts.test.tstools/scripts/test/ts.test.ts
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to packages/**/test/**/*.test.ts : Name test files as test/**/*.test.ts and run them with Vitest
Applied to files:
tools/scripts/test/ts.test.ts
📚 Learning: 2025-09-18T07:56:17.417Z
Learnt from: CR
PR: eggjs/egg#0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T07:56:17.417Z
Learning: For features affecting HTTP or process orchestration, add regression tests that exercise both the CommonJS and TypeScript example apps
Applied to files:
tools/scripts/test/ts.test.tstools/scripts/test/stop.test.ts
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to {packages,plugins,tools}/**/vitest.config.ts : Each Vitest-using package should include a vitest.config.ts
Applied to files:
tools/scripts/test/ts.test.tstools/scripts/vitest.config.ts
📚 Learning: 2025-09-18T07:56:17.417Z
Learnt from: CR
PR: eggjs/egg#0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T07:56:17.417Z
Learning: Applies to **/test/**/*.test.ts : Place test suites following Vitest discovery: **/test/**/*.test.ts
Applied to files:
tools/scripts/test/ts.test.ts
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to plugins/**/test/**/*.test.ts : Plugin tests must use Vitest (import from 'vitest') and follow the test file pattern test/**/*.test.ts
Applied to files:
tools/scripts/test/ts.test.ts
📚 Learning: 2025-09-18T07:56:17.417Z
Learnt from: CR
PR: eggjs/egg#0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T07:56:17.417Z
Learning: Applies to **/test/**/*.test.ts : Mirror the repository test pattern when adding new suites
Applied to files:
tools/scripts/test/ts.test.ts
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to packages/**/test/**/*.test.ts : Use Node.js built-in assert module for test assertions
Applied to files:
tools/scripts/test/ts.test.ts
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to packages/cookies/test/**/*.test.ts : The cookies package must use Mocha for tests with files under test/**/*.test.ts
Applied to files:
tools/scripts/test/ts.test.ts
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to plugins/**/package.json : All plugins must include egg as a peerDependency with version "workspace:*"
Applied to files:
tools/scripts/test/fixtures/custom-node-dir/package.jsontools/scripts/test/fixtures/cluster-config/package.jsontools/scripts/test/fixtures/egg-app/package.json
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to tsconfig.json : Root tsconfig.json must include all packages in the references array
Applied to files:
tsconfig.json
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to {packages,plugins,tools}/**/package.json : Each package must define scripts: typecheck as "tsc --noEmit" and lint as "oxlint --type-aware"
Applied to files:
tools/scripts/test/fixtures/ts/package.jsontools/scripts/package.json
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to plugins/**/tsdown.config.ts : All plugins must use the standard tsdown configuration template (entry: 'src/**/*.ts', dts: true, exports.devExports: true, unused.level: 'error')
Applied to files:
tools/scripts/tsdown.config.ts
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to {packages,plugins,tools}/**/tsdown.config.ts : Each package should use tsdown for builds and include a tsdown.config.ts
Applied to files:
tools/scripts/tsdown.config.ts
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to packages/*/tsdown.config.ts : Each package must provide a tsdown.config.ts for unbundled ESM builds
Applied to files:
tools/scripts/tsdown.config.ts
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to pnpm-workspace.yaml : Manage external dependency versions via the catalog in pnpm-workspace.yaml
Applied to files:
pnpm-workspace.yaml
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
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-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to {package.json,{packages,plugins,tools}/**/package.json} : Use workspace:* for internal dependencies and catalog: for external dependencies defined in pnpm-workspace.yaml
Applied to files:
pnpm-workspace.yaml
🧬 Code graph analysis (20)
tools/scripts/test/fixtures/ts-pkg/app/router.js (1)
tools/scripts/test/fixtures/ts/app/router.js (1)
app(4-4)
tools/scripts/test/fixtures/egg-scripts-config/app.js (1)
tools/scripts/test/fixtures/status/app.js (1)
require(1-1)
tools/scripts/test/fixtures/ts/app/controller/home.js (1)
tools/scripts/test/fixtures/ts/app/controller/home.ts (1)
AppController(3-15)
tools/scripts/scripts/start-cluster.cjs (1)
tools/scripts/scripts/start-cluster.mjs (3)
debug(4-4)options(8-8)importModule(10-10)
tools/scripts/src/helper.ts (1)
scripts/version.js (1)
__dirname(13-13)
tools/scripts/test/egg-scripts.test.ts (1)
tools/scripts/src/helper.ts (1)
getSourceFilename(62-64)
tools/scripts/test/fixtures/ts-pkg/app/controller/home.ts (1)
tools/scripts/test/fixtures/ts/app/controller/home.ts (1)
AppController(3-15)
tools/scripts/test/ts.test.ts (2)
tools/scripts/src/helper.ts (2)
getSourceFilename(62-64)isWindows(5-5)tools/scripts/test/utils.ts (3)
Coffee(6-6)cleanup(8-42)replaceWeakRefMessage(44-50)
tools/scripts/test/fixtures/ts/app/router.js (1)
tools/scripts/test/fixtures/ts-pkg/app/router.js (1)
app(4-4)
tools/scripts/src/commands/stop.ts (1)
tools/scripts/src/helper.ts (4)
isWindows(5-5)NodeProcess(9-12)findNodeProcess(16-38)kill(40-50)
tools/scripts/test/fixtures/example/app/router.js (2)
tools/scripts/test/fixtures/ts-pkg/app/router.js (1)
app(4-4)tools/scripts/test/fixtures/ts/app/router.js (1)
app(4-4)
tools/scripts/scripts/start-cluster.mjs (1)
tools/scripts/scripts/start-cluster.cjs (3)
debug(5-5)options(9-9)importModule(11-11)
tools/scripts/src/baseCommand.ts (1)
tools/scripts/src/types.ts (1)
PackageEgg(1-9)
tools/scripts/vitest.config.ts (1)
packages/egg/src/lib/types.ts (1)
defineConfig(106-108)
tools/scripts/test/fixtures/ts-pkg/app/controller/home.js (2)
tools/scripts/test/fixtures/ts-pkg/app/controller/home.ts (1)
AppController(3-15)tools/scripts/test/fixtures/ts/app/controller/home.ts (1)
AppController(3-15)
tools/scripts/test/start.test.ts (2)
tools/scripts/src/helper.ts (2)
getSourceFilename(62-64)isWindows(5-5)tools/scripts/test/utils.ts (3)
Coffee(6-6)cleanup(8-42)replaceWeakRefMessage(44-50)
tools/scripts/test/stop.test.ts (2)
tools/scripts/src/helper.ts (2)
getSourceFilename(62-64)isWindows(5-5)tools/scripts/test/utils.ts (3)
Coffee(6-6)cleanup(8-42)replaceWeakRefMessage(44-50)
tools/scripts/test/fixtures/status/app.js (1)
tools/scripts/test/fixtures/egg-scripts-config/app.js (1)
require(1-1)
tools/scripts/src/commands/start.ts (4)
tools/scripts/scripts/start-cluster.cjs (2)
debug(5-5)options(9-9)tools/scripts/scripts/start-cluster.mjs (2)
debug(4-4)options(8-8)tools/scripts/src/helper.ts (1)
getSourceDirname(52-60)packages/utils/src/import.ts (1)
importResolve(275-361)
tools/scripts/test/utils.ts (1)
tools/scripts/src/helper.ts (2)
findNodeProcess(16-38)isWindows(5-5)
🪛 ast-grep (0.39.5)
tools/scripts/test/stop.test.ts
[warning] 358-358: 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(Starting custom-framework application at ${appPathInRegexp})
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)
🪛 markdownlint-cli2 (0.18.1)
tools/scripts/README.md
19-19: Bare URL used
(MD034, no-bare-urls)
30-30: Bare URL used
(MD034, no-bare-urls)
32-32: Bare URL used
(MD034, no-bare-urls)
42-42: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
42-42: Hard tabs
Column: 1
(MD010, no-hard-tabs)
43-43: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
43-43: Hard tabs
Column: 1
(MD010, no-hard-tabs)
44-44: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
44-44: Hard tabs
Column: 1
(MD010, no-hard-tabs)
45-45: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
45-45: Hard tabs
Column: 1
(MD010, no-hard-tabs)
46-46: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
46-46: Hard tabs
Column: 1
(MD010, no-hard-tabs)
49-49: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
49-49: Hard tabs
Column: 1
(MD010, no-hard-tabs)
50-50: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
50-50: Hard tabs
Column: 1
(MD010, no-hard-tabs)
51-51: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
51-51: Hard tabs
Column: 1
(MD010, no-hard-tabs)
52-52: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
52-52: Hard tabs
Column: 1
(MD010, no-hard-tabs)
57-57: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
57-57: Hard tabs
Column: 1
(MD010, no-hard-tabs)
58-58: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
58-58: Hard tabs
Column: 1
(MD010, no-hard-tabs)
59-59: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
59-59: Hard tabs
Column: 1
(MD010, no-hard-tabs)
62-62: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
62-62: Hard tabs
Column: 1
(MD010, no-hard-tabs)
63-63: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
63-63: Hard tabs
Column: 1
(MD010, no-hard-tabs)
100-100: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
103-103: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
104-104: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
106-106: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
109-109: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
110-110: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
112-112: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
115-115: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
116-116: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
118-118: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
119-119: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
tools/scripts/CHANGELOG.md
19-19: Bare URL used
(MD034, no-bare-urls)
30-30: Bare URL used
(MD034, no-bare-urls)
32-32: Bare URL used
(MD034, no-bare-urls)
42-42: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
42-42: Hard tabs
Column: 1
(MD010, no-hard-tabs)
43-43: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
43-43: Hard tabs
Column: 1
(MD010, no-hard-tabs)
44-44: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
44-44: Hard tabs
Column: 1
(MD010, no-hard-tabs)
45-45: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
45-45: Hard tabs
Column: 1
(MD010, no-hard-tabs)
46-46: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
46-46: Hard tabs
Column: 1
(MD010, no-hard-tabs)
49-49: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
49-49: Hard tabs
Column: 1
(MD010, no-hard-tabs)
50-50: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
50-50: Hard tabs
Column: 1
(MD010, no-hard-tabs)
51-51: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
51-51: Hard tabs
Column: 1
(MD010, no-hard-tabs)
52-52: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
52-52: Hard tabs
Column: 1
(MD010, no-hard-tabs)
57-57: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
57-57: Hard tabs
Column: 1
(MD010, no-hard-tabs)
58-58: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
58-58: Hard tabs
Column: 1
(MD010, no-hard-tabs)
59-59: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
59-59: Hard tabs
Column: 1
(MD010, no-hard-tabs)
62-62: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
62-62: Hard tabs
Column: 1
(MD010, no-hard-tabs)
63-63: Unordered list indentation
Expected: 2; Actual: 1
(MD007, ul-indent)
63-63: Hard tabs
Column: 1
(MD010, no-hard-tabs)
100-100: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
103-103: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
104-104: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
106-106: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
109-109: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
110-110: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
112-112: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
115-115: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
116-116: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
118-118: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
119-119: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
121-121: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
124-124: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
125-125: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
127-127: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
128-128: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
130-130: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
133-133: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
134-134: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
136-136: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
137-137: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
139-139: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
142-142: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
143-143: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
145-145: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
148-148: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
149-149: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
151-151: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
154-154: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
155-155: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
157-157: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
160-160: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
161-161: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
163-163: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
166-166: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
167-167: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
169-169: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
170-170: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
172-172: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
175-175: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
176-176: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
178-178: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
181-181: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
182-182: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
184-184: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
185-185: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
186-186: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
188-188: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
191-191: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
193-193: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
196-196: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
197-197: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
199-199: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
200-200: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
202-202: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
205-205: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
206-206: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
208-208: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
211-211: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
212-212: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
214-214: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
217-217: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
218-218: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
220-220: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
221-221: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
223-223: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
226-226: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
228-228: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
231-231: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
233-233: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
236-236: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
237-237: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
239-239: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
242-242: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
243-243: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
245-245: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
246-246: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
248-248: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
251-251: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
252-252: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
254-254: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
257-257: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
258-258: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
260-260: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
263-263: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
264-264: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
266-266: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
269-269: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
270-270: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
272-272: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
275-275: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
276-276: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
278-278: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
279-279: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
281-281: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
284-284: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
285-285: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
287-287: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
288-288: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
290-290: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
293-293: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
294-294: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
296-296: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
299-299: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
301-301: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
304-304: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
306-306: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
309-309: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
⏰ 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). (2)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: Cloudflare Pages: egg-v3
| async function main() { | ||
| debug('argv: %o', process.argv); | ||
| const options = JSON.parse(process.argv[2]); | ||
| debug('start cluster options: %o', options); | ||
| const { startCluster } = await importModule(options.framework); | ||
| await startCluster(options); | ||
| } |
There was a problem hiding this comment.
Add error handling for JSON parsing and cluster start failures.
The main() function lacks error handling, which can lead to uncaught exceptions and silent failures:
JSON.parse(process.argv[2])will throw if the argument is missing or malformed JSONstartCluster(options)failures won't be caught or logged- The
void main()pattern on line 15 ignores the promise, suppressing any errors
Apply this diff to add proper error handling:
async function main() {
- debug('argv: %o', process.argv);
- const options = JSON.parse(process.argv[2]);
- debug('start cluster options: %o', options);
- const { startCluster } = await importModule(options.framework);
- await startCluster(options);
+ try {
+ debug('argv: %o', process.argv);
+ if (!process.argv[2]) {
+ throw new Error('Missing options argument');
+ }
+ const options = JSON.parse(process.argv[2]);
+ debug('start cluster options: %o', options);
+ const { startCluster } = await importModule(options.framework);
+ await startCluster(options);
+ } catch (err) {
+ console.error('Failed to start cluster:', err);
+ process.exit(1);
+ }
}
-void main();
+main().catch(err => {
+ console.error('Unhandled error:', err);
+ process.exit(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.
| async function main() { | |
| debug('argv: %o', process.argv); | |
| const options = JSON.parse(process.argv[2]); | |
| debug('start cluster options: %o', options); | |
| const { startCluster } = await importModule(options.framework); | |
| await startCluster(options); | |
| } | |
| async function main() { | |
| try { | |
| debug('argv: %o', process.argv); | |
| if (!process.argv[2]) { | |
| throw new Error('Missing options argument'); | |
| } | |
| const options = JSON.parse(process.argv[2]); | |
| debug('start cluster options: %o', options); | |
| const { startCluster } = await importModule(options.framework); | |
| await startCluster(options); | |
| } catch (err) { | |
| console.error('Failed to start cluster:', err); | |
| process.exit(1); | |
| } | |
| } | |
| main().catch(err => { | |
| console.error('Unhandled error:', err); | |
| process.exit(1); | |
| }); |
🤖 Prompt for AI Agents
In tools/scripts/scripts/start-cluster.cjs around lines 7 to 13, add robust
error handling: validate that process.argv[2] exists and wrap JSON.parse in a
try/catch to log a clear error and exit non‑zero on malformed or missing JSON;
wrap the dynamic import and startCluster(options) call in a try/catch to log any
import/start failures and exit non‑zero; ensure the top‑level invocation does
not use void main() but instead awaits or attaches .catch(...) to main() so
unhandled rejections are logged and the process exits with a non‑zero code.
| export async function findNodeProcess(filterFn?: FilterFunction): Promise<NodeProcess[]> { | ||
| const command = isWindows | ||
| ? 'wmic Path win32_process Where "Name = \'node.exe\'" Get CommandLine,ProcessId' | ||
| : // command, cmd are alias of args, not POSIX standard, so we use args | ||
| 'ps -wweo "pid,args"'; | ||
| const stdio = await runScript(command, { stdio: 'pipe' }); | ||
| const processList = stdio | ||
| .stdout!.toString() | ||
| .split('\n') | ||
| .reduce<NodeProcess[]>((arr, line) => { | ||
| if (!!line && !line.includes('/bin/sh') && line.includes('node')) { | ||
| const m = line.match(REGEX); | ||
| if (m) { | ||
| const item: NodeProcess = isWindows ? { pid: parseInt(m[2]), cmd: m[1] } : { pid: parseInt(m[1]), cmd: m[2] }; | ||
| if (filterFn?.(item)) { | ||
| arr.push(item); | ||
| } | ||
| } | ||
| } | ||
| return arr; | ||
| }, []); | ||
| return processList; | ||
| } |
There was a problem hiding this comment.
Add defensive check for stdout.
Line 23 uses a non-null assertion (stdout!.toString()) assuming stdout is always present. While runScript with stdio: 'pipe' should populate stdout, defensive coding would prevent potential runtime crashes.
Apply this diff to add a defensive check:
const stdio = await runScript(command, { stdio: 'pipe' });
+ if (!stdio.stdout) {
+ return [];
+ }
const processList = stdio
- .stdout!.toString()
+ .stdout.toString()
.split('\n')📝 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 async function findNodeProcess(filterFn?: FilterFunction): Promise<NodeProcess[]> { | |
| const command = isWindows | |
| ? 'wmic Path win32_process Where "Name = \'node.exe\'" Get CommandLine,ProcessId' | |
| : // command, cmd are alias of args, not POSIX standard, so we use args | |
| 'ps -wweo "pid,args"'; | |
| const stdio = await runScript(command, { stdio: 'pipe' }); | |
| const processList = stdio | |
| .stdout!.toString() | |
| .split('\n') | |
| .reduce<NodeProcess[]>((arr, line) => { | |
| if (!!line && !line.includes('/bin/sh') && line.includes('node')) { | |
| const m = line.match(REGEX); | |
| if (m) { | |
| const item: NodeProcess = isWindows ? { pid: parseInt(m[2]), cmd: m[1] } : { pid: parseInt(m[1]), cmd: m[2] }; | |
| if (filterFn?.(item)) { | |
| arr.push(item); | |
| } | |
| } | |
| } | |
| return arr; | |
| }, []); | |
| return processList; | |
| } | |
| export async function findNodeProcess(filterFn?: FilterFunction): Promise<NodeProcess[]> { | |
| const command = isWindows | |
| ? 'wmic Path win32_process Where "Name = \'node.exe\'" Get CommandLine,ProcessId' | |
| : // command, cmd are alias of args, not POSIX standard, so we use args | |
| 'ps -wweo "pid,args"'; | |
| const stdio = await runScript(command, { stdio: 'pipe' }); | |
| if (!stdio.stdout) { | |
| return []; | |
| } | |
| const processList = stdio | |
| .stdout.toString() | |
| .split('\n') | |
| .reduce<NodeProcess[]>((arr, line) => { | |
| if (!!line && !line.includes('/bin/sh') && line.includes('node')) { | |
| const m = line.match(REGEX); | |
| if (m) { | |
| const item: NodeProcess = isWindows | |
| ? { pid: parseInt(m[2]), cmd: m[1] } | |
| : { pid: parseInt(m[1]), cmd: m[2] }; | |
| if (filterFn?.(item)) { | |
| arr.push(item); | |
| } | |
| } | |
| } | |
| return arr; | |
| }, []); | |
| return processList; | |
| } |
🤖 Prompt for AI Agents
In tools/scripts/src/helper.ts around lines 16 to 38, the code uses a non-null
assertion stdout!.toString() which can throw if stdout is undefined; change to
defensively handle missing stdout by first checking stdio.stdout — if it's
null/undefined return an empty array (or otherwise treat output as empty string)
before attempting to call toString(), then continue parsing; ensure the function
returns [] when no stdout is present to avoid runtime crashes.
| { | ||
| "name": "example", | ||
| "version": "1.0.0", | ||
| "dependencies": { | ||
| "egg": "^1.0.0" | ||
| } | ||
| } |
There was a problem hiding this comment.
Include engines + lint/typecheck scripts to satisfy workspace policy.
This fixture package.json also misses the required "engines": { "node": ">=22.18.0" } declaration and the standard typecheck / lint scripts ("tsc --noEmit", "oxlint --type-aware"). Please add them so it complies with the workspace rules. [Based on coding guidelines]
🤖 Prompt for AI Agents
In tools/scripts/test/fixtures/cluster-config/package.json lines 1-7, the
package.json is missing the required engines declaration and standard
lint/typecheck scripts; add an "engines" field with "node": ">=22.18.0" and add
a "scripts" object that includes "typecheck": "tsc --noEmit" and "lint": "oxlint
--type-aware" (place scripts at the top-level of the JSON alongside
name/version/dependencies) so the fixture complies with workspace policy.
| app.beforeStart(async () => { | ||
| await scheduler.wait(parseInt(process.env.WAIT_TIME)); | ||
| }); |
There was a problem hiding this comment.
Validate or provide a default for WAIT_TIME.
parseInt(process.env.WAIT_TIME) returns NaN if WAIT_TIME is undefined, which could cause unexpected behavior in scheduler.wait().
Apply this diff to add validation:
app.beforeStart(async () => {
- await scheduler.wait(parseInt(process.env.WAIT_TIME));
+ await scheduler.wait(parseInt(process.env.WAIT_TIME) || 0);
});📝 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.
| app.beforeStart(async () => { | |
| await scheduler.wait(parseInt(process.env.WAIT_TIME)); | |
| }); | |
| app.beforeStart(async () => { | |
| await scheduler.wait(parseInt(process.env.WAIT_TIME) || 0); | |
| }); |
🤖 Prompt for AI Agents
In tools/scripts/test/fixtures/egg-scripts-config/app.js around lines 8 to 10,
parseInt(process.env.WAIT_TIME) can produce NaN when WAIT_TIME is undefined or
invalid; change to parse the value with an explicit radix and a safe default,
e.g., compute const wait = Number.parseInt(process.env.WAIT_TIME ?? '0', 10),
then if Number.isNaN(wait) assign a fallback (or throw/log and exit) before
calling scheduler.wait(wait) so scheduler.wait() always receives a valid number.
| { | ||
| "name": "trace-warnings", | ||
| "version": "1.0.0" | ||
| } |
There was a problem hiding this comment.
Add required engines and lint/typecheck scripts
Per the repo guidelines, every package.json under tools/** must set engines.node >= 22.18.0 and define typecheck/lint scripts. Please extend this fixture accordingly (keeping it private helps avoid accidental publishes).
{
"name": "trace-warnings",
- "version": "1.0.0"
+ "version": "1.0.0",
+ "private": true,
+ "engines": {
+ "node": ">=22.18.0"
+ },
+ "scripts": {
+ "typecheck": "tsc --noEmit",
+ "lint": "oxlint --type-aware"
+ }
}As per coding guidelines
📝 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.
| { | |
| "name": "trace-warnings", | |
| "version": "1.0.0" | |
| } | |
| { | |
| "name": "trace-warnings", | |
| "version": "1.0.0", | |
| "private": true, | |
| "engines": { | |
| "node": ">=22.18.0" | |
| }, | |
| "scripts": { | |
| "typecheck": "tsc --noEmit", | |
| "lint": "oxlint --type-aware" | |
| } | |
| } |
🤖 Prompt for AI Agents
In tools/scripts/test/fixtures/trace-warnings/package.json around lines 1 to 4,
the package.json is missing required fields per repo guidelines: add an
"engines" entry setting "node": ">=22.18.0", ensure "private": true remains (or
add it) to prevent accidental publishes, and add "scripts" with at minimum
"typecheck" and "lint" commands (e.g., point "typecheck" to your
monorepo/typecheck runner or "tsc --noEmit" and "lint" to the repo linter
command). Make sure the new fields are valid JSON and placed at the top level of
the package.json.
| "strict": true, | ||
| "noImplicitAny": false, |
There was a problem hiding this comment.
Resolve strict mode inconsistency.
Setting strict: true enables all strict family checks including noImplicitAny, but line 7 explicitly disables it. This creates an inconsistent configuration that weakens type safety.
Apply this diff to resolve the inconsistency:
"strict": true,
- "noImplicitAny": false,
"experimentalDecorators": true,If you need to allow implicit any for this fixture, consider removing strict: true and enabling only the specific strict checks you need.
📝 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.
| "strict": true, | |
| "noImplicitAny": false, | |
| "strict": true, | |
| "experimentalDecorators": true, |
🤖 Prompt for AI Agents
In tools/scripts/test/fixtures/ts/tsconfig.json around lines 6 to 7, the config
sets "strict": true while also setting "noImplicitAny": false which is
inconsistent; either remove the explicit "noImplicitAny": false (so all strict
checks remain enabled) or disable the umbrella strict flag and explicitly enable
the needed strict options—update the file to choose one approach so
"noImplicitAny" aligns with "strict".
| process.kill(pid, type === 'master' ? '' : 'SIGKILL'); | ||
| console.log(`cleanup ${type} ${pid}`); |
There was a problem hiding this comment.
Fix invalid signal passed to process.kill.
process.kill(pid, '') throws ERR_INVALID_SIGNAL on Node (empty string isn’t a valid signal). Call process.kill(pid) for the master path or use an explicit signal like 'SIGTERM' instead.
- process.kill(pid, type === 'master' ? '' : 'SIGKILL');
+ if (type === 'master') {
+ process.kill(pid);
+ } else {
+ process.kill(pid, 'SIGKILL');
+ }📝 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.
| process.kill(pid, type === 'master' ? '' : 'SIGKILL'); | |
| console.log(`cleanup ${type} ${pid}`); | |
| if (type === 'master') { | |
| process.kill(pid); | |
| } else { | |
| process.kill(pid, 'SIGKILL'); | |
| } | |
| console.log(`cleanup ${type} ${pid}`); |
🤖 Prompt for AI Agents
In tools/scripts/test/utils.ts around lines 30-31, the code calls
process.kill(pid, type === 'master' ? '' : 'SIGKILL') which passes an invalid
empty-string signal for the master path; change it to call process.kill(pid)
when type === 'master' and keep process.kill(pid, 'SIGKILL') for non-master,
i.e. branch so master uses process.kill(pid) (no second arg) and others use the
explicit 'SIGKILL' signal.
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #5591 +/- ##
===========================================
+ Coverage 80.67% 91.76% +11.09%
===========================================
Files 212 3 -209
Lines 6017 85 -5932
Branches 878 14 -864
===========================================
- Hits 4854 78 -4776
+ Misses 1088 7 -1081
+ Partials 75 0 -75 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
tools/scripts/scripts/start-cluster.cjs (2)
7-17: Good: top-level error handling is now in place.The past review concern about unhandled promise rejections has been addressed by the
.catch()handler on lines 19-22. However, consider these optional improvements:
Validate
process.argv[2]before parsing (line 9): If the argument is missing,JSON.parse(undefined)throws an unclear error ("Unexpected token u..."). Adding a validation check would provide a clearer error message.Verify the fallback logic intentionality (lines 11-15): The CommonJS version checks
exports.startClusterand falls back toexports.default.startCluster, whereas the ESM variant (start-cluster.mjs:10) directly destructures{ startCluster }. If this fallback is intentional for CommonJS module compatibility, it's fine; otherwise, consider aligning with the ESM approach.Optional improvement for clearer error messages:
async function main() { debug('argv: %o', process.argv); + if (!process.argv[2]) { + throw new Error('Missing options argument: expected JSON options as argv[2]'); + } const options = JSON.parse(process.argv[2]); debug('start cluster options: %o', options); const exports = await importModule(options.framework); let startCluster = exports.startCluster; if (typeof startCluster !== 'function') { startCluster = exports.default.startCluster; } + if (typeof startCluster !== 'function') { + throw new Error('startCluster function not found in framework module'); + } await startCluster(options); }
19-22: Error handling is correctly implemented.The
.catch()handler properly logs errors and exits with a non-zero code. Optionally, consider adding a descriptive prefix to the error message for clarity.Optional improvement:
main().catch(err => { - console.error(err); + console.error('Failed to start cluster:', err); process.exit(1); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (29)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamltools/scripts/test/fixtures/egg-app/node_modules/egg/index.jsis excluded by!**/node_modules/**tools/scripts/test/fixtures/egg-app/node_modules/egg/package.jsonis excluded by!**/node_modules/**tools/scripts/test/fixtures/egg-revert/node_modules/custom-framework/index.jsis excluded by!**/node_modules/**tools/scripts/test/fixtures/egg-revert/node_modules/custom-framework/package.jsonis excluded by!**/node_modules/**tools/scripts/test/fixtures/egg-scripts-config/node_modules/custom-framework/index.jsis excluded by!**/node_modules/**tools/scripts/test/fixtures/egg-scripts-config/node_modules/custom-framework/package.jsonis excluded by!**/node_modules/**tools/scripts/test/fixtures/example-stop/node_modules/custom-framework/index.jsis excluded by!**/node_modules/**tools/scripts/test/fixtures/example-stop/node_modules/custom-framework/package.jsonis excluded by!**/node_modules/**tools/scripts/test/fixtures/example-stop/node_modules/yadan/config/config.default.jsis excluded by!**/node_modules/**tools/scripts/test/fixtures/example-stop/node_modules/yadan/index.jsis excluded by!**/node_modules/**tools/scripts/test/fixtures/example-stop/node_modules/yadan/package.jsonis excluded by!**/node_modules/**tools/scripts/test/fixtures/example/node_modules/custom-framework/index.jsis excluded by!**/node_modules/**tools/scripts/test/fixtures/example/node_modules/custom-framework/package.jsonis excluded by!**/node_modules/**tools/scripts/test/fixtures/example/node_modules/yadan/config/config.default.jsis excluded by!**/node_modules/**tools/scripts/test/fixtures/example/node_modules/yadan/index.jsis excluded by!**/node_modules/**tools/scripts/test/fixtures/example/node_modules/yadan/package.jsonis excluded by!**/node_modules/**tools/scripts/test/fixtures/pkg-config-esm/node_modules/custom-framework/index.jsis excluded by!**/node_modules/**tools/scripts/test/fixtures/pkg-config-esm/node_modules/custom-framework/package.jsonis excluded by!**/node_modules/**tools/scripts/test/fixtures/pkg-config-esm/node_modules/inject/index.jsis excluded by!**/node_modules/**tools/scripts/test/fixtures/pkg-config-esm/node_modules/inject/package.jsonis excluded by!**/node_modules/**tools/scripts/test/fixtures/pkg-config-sourcemap/node_modules/custom-framework/index.jsis excluded by!**/node_modules/**tools/scripts/test/fixtures/pkg-config-sourcemap/node_modules/custom-framework/package.jsonis excluded by!**/node_modules/**tools/scripts/test/fixtures/pkg-config-sourcemap/node_modules/inject/index.jsis excluded by!**/node_modules/**tools/scripts/test/fixtures/pkg-config/node_modules/custom-framework/index.jsis excluded by!**/node_modules/**tools/scripts/test/fixtures/pkg-config/node_modules/custom-framework/package.jsonis excluded by!**/node_modules/**tools/scripts/test/fixtures/pkg-config/node_modules/inject/index.jsis excluded by!**/node_modules/**tools/scripts/test/fixtures/status/node_modules/custom-framework/index.jsis excluded by!**/node_modules/**tools/scripts/test/fixtures/status/node_modules/custom-framework/package.jsonis excluded by!**/node_modules/**
📒 Files selected for processing (29)
.github/workflows/ci.yml(1 hunks)package.json(1 hunks)pnpm-workspace.yaml(3 hunks)tools/egg-bin/vitest.config.ts(0 hunks)tools/scripts/.gitignore(1 hunks)tools/scripts/package.json(1 hunks)tools/scripts/scripts/start-cluster.cjs(1 hunks)tools/scripts/scripts/start-cluster.mjs(1 hunks)tools/scripts/src/baseCommand.ts(1 hunks)tools/scripts/src/commands/start.ts(1 hunks)tools/scripts/src/commands/stop.ts(1 hunks)tools/scripts/src/helper.ts(1 hunks)tools/scripts/src/index.ts(1 hunks)tools/scripts/test/egg-scripts.test.ts(1 hunks)tools/scripts/test/fixtures/example-stop/app.js(1 hunks)tools/scripts/test/fixtures/example-stop/app/router.js(1 hunks)tools/scripts/test/fixtures/example-stop/config/config.default.js(1 hunks)tools/scripts/test/fixtures/example-stop/config/config.pre.js(1 hunks)tools/scripts/test/fixtures/example-stop/package.json(1 hunks)tools/scripts/test/fixtures/ts/package.json(1 hunks)tools/scripts/test/start-without-demon.test.ts(1 hunks)tools/scripts/test/start.test.ts(1 hunks)tools/scripts/test/stop.test.ts(1 hunks)tools/scripts/test/ts.test.ts(1 hunks)tools/scripts/test/utils.ts(1 hunks)tools/scripts/tsconfig.json(1 hunks)tools/scripts/tsdown.config.ts(1 hunks)tools/scripts/vitest.config.ts(1 hunks)vitest.config.ts(1 hunks)
💤 Files with no reviewable changes (1)
- tools/egg-bin/vitest.config.ts
✅ Files skipped from review due to trivial changes (2)
- tools/scripts/.gitignore
- tools/scripts/test/fixtures/example-stop/config/config.pre.js
🚧 Files skipped from review as they are similar to previous changes (8)
- tools/scripts/src/helper.ts
- tools/scripts/test/fixtures/ts/package.json
- tools/scripts/tsconfig.json
- tools/scripts/scripts/start-cluster.mjs
- tools/scripts/tsdown.config.ts
- tools/scripts/package.json
- tools/scripts/test/start.test.ts
- tools/scripts/test/utils.ts
🧰 Additional context used
📓 Path-based instructions (7)
{package.json,{packages,plugins,tools}/**/package.json}
📄 CodeRabbit inference engine (CLAUDE.md)
{package.json,{packages,plugins,tools}/**/package.json}: All packages must require Node.js >= 22.18.0 (set engines.node accordingly)
Use workspace:* for internal dependencies and catalog: for external dependencies defined in pnpm-workspace.yaml
Files:
package.jsontools/scripts/test/fixtures/example-stop/package.json
**/test/fixtures/**
📄 CodeRabbit inference engine (AGENTS.md)
Put reusable test data under test/fixtures/
Files:
tools/scripts/test/fixtures/example-stop/app/router.jstools/scripts/test/fixtures/example-stop/app.jstools/scripts/test/fixtures/example-stop/config/config.default.jstools/scripts/test/fixtures/example-stop/package.json
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Prefer TypeScript and ESM: write sources and exports in .ts (ESM-first) rather than CommonJS
Use two-space indentation, trailing commas, and semicolons (Prettier/oxlint defaults)
Name files in lowercase with hyphens (e.g., loader-context.ts)
Name classes in PascalCase
Name functions and variables in camelCase
Re-export types thoughtfully to keep the public API stable
Files:
tools/scripts/src/index.tstools/scripts/test/ts.test.tstools/scripts/test/start-without-demon.test.tsvitest.config.tstools/scripts/src/commands/stop.tstools/scripts/src/commands/start.tstools/scripts/test/egg-scripts.test.tstools/scripts/vitest.config.tstools/scripts/src/baseCommand.tstools/scripts/test/stop.test.ts
**/test/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/test/**/*.test.ts: Place test suites following Vitest discovery: /test//*.test.ts
Mirror the repository test pattern when adding new suitesTest files should be named with the pattern test/**/*.test.ts
Files:
tools/scripts/test/ts.test.tstools/scripts/test/start-without-demon.test.tstools/scripts/test/egg-scripts.test.tstools/scripts/test/stop.test.ts
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:"
Manage external dependency versions via the catalog in pnpm-workspace.yaml
Files:
pnpm-workspace.yaml
{packages,plugins,tools}/**/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
Each package must define scripts: typecheck as "tsc --noEmit" and lint as "oxlint --type-aware"
Files:
tools/scripts/test/fixtures/example-stop/package.json
{packages,plugins,tools}/**/vitest.config.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Each Vitest-using package should include a vitest.config.ts
Files:
tools/scripts/vitest.config.ts
🧠 Learnings (12)
📚 Learning: 2025-09-18T07:56:17.417Z
Learnt from: CR
PR: eggjs/egg#0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T07:56:17.417Z
Learning: Run pnpm run test locally before submitting changes
Applied to files:
package.json
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to packages/{egg,core,utils,mock,cluster,koa,supertest,extend2,koa-static-cache,router}/test/**/*.test.ts : Core packages must use Vitest (import from 'vitest') with test files under test/**/*.test.ts
Applied to files:
tools/scripts/test/ts.test.tsvitest.config.tstools/scripts/test/egg-scripts.test.tstools/scripts/vitest.config.tstools/scripts/test/stop.test.ts
📚 Learning: 2025-09-18T07:56:17.417Z
Learnt from: CR
PR: eggjs/egg#0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T07:56:17.417Z
Learning: For features affecting HTTP or process orchestration, add regression tests that exercise both the CommonJS and TypeScript example apps
Applied to files:
tools/scripts/test/ts.test.tstools/scripts/test/stop.test.ts
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to packages/**/test/**/*.test.ts : Name test files as test/**/*.test.ts and run them with Vitest
Applied to files:
tools/scripts/test/ts.test.tstools/scripts/vitest.config.ts
📚 Learning: 2025-09-18T07:56:17.417Z
Learnt from: CR
PR: eggjs/egg#0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T07:56:17.417Z
Learning: Applies to **/test/**/*.test.ts : Place test suites following Vitest discovery: **/test/**/*.test.ts
Applied to files:
tools/scripts/test/ts.test.ts
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to {packages,plugins,tools}/**/vitest.config.ts : Each Vitest-using package should include a vitest.config.ts
Applied to files:
tools/scripts/test/ts.test.tsvitest.config.tstools/scripts/vitest.config.ts
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to tools/egg-bin/test/**/*.test.ts : The egg-bin tool package must use Mocha for tests with files under test/**/*.test.ts
Applied to files:
tools/scripts/test/ts.test.tstools/scripts/test/start-without-demon.test.tsvitest.config.tstools/scripts/test/egg-scripts.test.ts
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to packages/**/test/**/*.test.ts : Use import { describe, it } from 'vitest' in tests
Applied to files:
tools/scripts/test/ts.test.tstools/scripts/test/egg-scripts.test.tstools/scripts/vitest.config.ts
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to plugins/**/test/**/*.test.ts : Plugin tests must use Vitest (import from 'vitest') and follow the test file pattern test/**/*.test.ts
Applied to files:
tools/scripts/test/ts.test.tstools/scripts/vitest.config.ts
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to packages/**/test/**/*.test.ts : Use Node.js built-in assert module for test assertions
Applied to files:
tools/scripts/test/ts.test.ts
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to packages/cookies/test/**/*.test.ts : The cookies package must use Mocha for tests with files under test/**/*.test.ts
Applied to files:
tools/scripts/test/ts.test.ts
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to pnpm-workspace.yaml : Manage external dependency versions via the catalog in pnpm-workspace.yaml
Applied to files:
pnpm-workspace.yaml
🧬 Code graph analysis (9)
tools/scripts/scripts/start-cluster.cjs (1)
tools/scripts/scripts/start-cluster.mjs (3)
debug(5-5)options(9-9)importModule(11-11)
tools/scripts/test/ts.test.ts (3)
packages/egg/test/utils.ts (2)
mm(23-23)restore(29-29)tools/scripts/test/utils.ts (2)
Coffee(8-8)cleanup(10-44)tools/scripts/src/helper.ts (1)
isWindows(3-3)
tools/scripts/test/fixtures/example-stop/config/config.default.js (1)
tools/scripts/scripts/start-cluster.cjs (1)
exports(11-11)
tools/scripts/test/start-without-demon.test.ts (3)
packages/egg/test/utils.ts (2)
mm(23-23)restore(29-29)tools/scripts/test/utils.ts (3)
Coffee(8-8)cleanup(10-44)replaceWeakRefMessage(46-52)tools/scripts/src/helper.ts (1)
isWindows(3-3)
tools/scripts/src/commands/stop.ts (1)
tools/scripts/src/helper.ts (4)
isWindows(3-3)NodeProcess(7-10)findNodeProcess(14-36)kill(38-48)
tools/scripts/src/commands/start.ts (3)
tools/scripts/scripts/start-cluster.cjs (2)
debug(5-5)options(9-9)tools/scripts/scripts/start-cluster.mjs (2)
debug(5-5)options(9-9)packages/utils/src/import.ts (1)
importResolve(275-361)
tools/scripts/test/egg-scripts.test.ts (1)
tools/egg-bin/test/commands/test.test.ts (1)
eggBin(8-436)
tools/scripts/src/baseCommand.ts (1)
tools/scripts/src/types.ts (1)
PackageEgg(1-9)
tools/scripts/test/stop.test.ts (2)
tools/scripts/test/utils.ts (3)
Coffee(8-8)cleanup(10-44)replaceWeakRefMessage(46-52)tools/scripts/src/helper.ts (1)
isWindows(3-3)
🪛 ast-grep (0.39.5)
tools/scripts/test/stop.test.ts
[warning] 390-390: 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(Starting custom-framework application at ${appPathInRegexp})
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). (18)
- GitHub Check: Test (macos-latest, 22, plugins)
- GitHub Check: Test (ubuntu-latest, 24, plugins)
- GitHub Check: Test (macos-latest, 24, packages)
- GitHub Check: Test (macos-latest, 22, packages)
- GitHub Check: Test (windows-latest, 24, tools)
- GitHub Check: Test (ubuntu-latest, 24, packages)
- GitHub Check: Test (ubuntu-latest, 22, packages)
- GitHub Check: Test (windows-latest, 22, packages)
- GitHub Check: Test (windows-latest, 24, packages)
- GitHub Check: Test (windows-latest, 22, tools)
- GitHub Check: Test (macos-latest, 24, plugins)
- GitHub Check: Test (windows-latest, 22, plugins)
- GitHub Check: Test (ubuntu-latest, 22, plugins)
- GitHub Check: Test (windows-latest, 24, plugins)
- GitHub Check: Test egg-bin (ubuntu-latest, 22)
- GitHub Check: Test egg-bin (windows-latest, 22)
- GitHub Check: typecheck
- GitHub Check: Cloudflare Pages: egg-v3
🔇 Additional comments (5)
tools/scripts/scripts/start-cluster.cjs (1)
1-5: LGTM!The imports and debug setup are appropriate for a CommonJS cluster entry point script.
.github/workflows/ci.yml (1)
182-214: Job structure looks solid.Thanks for wiring the dedicated
test-egg-scriptsjob with the same concurrency/caching patterns as the existing suites—makes the new package slot neatly into CI.tools/scripts/test/fixtures/example-stop/app.js (1)
1-4: Fixture captures the--no-deprecationscenario cleanly.Keeping the raw
new Buffercall here ensures the stop-flow tests can exercise the deprecation flag logic without touching production code—looks good.tools/scripts/vitest.config.ts (1)
1-9: Per-package Vitest config looks correct.Timeouts and glob selection line up with the new tools/scripts test suite—no issues spotted.
tools/scripts/test/egg-scripts.test.ts (1)
1-30: CLI help regression test looks solid.Nice Vitest migration—these coffee assertions cover the top-level, start, and stop help text exactly as the new CLI emits it.
| import Start from './commands/start.ts'; | ||
|
|
||
| // exports.StopCommand = require('./lib/cmd/stop'); | ||
|
|
||
| export * from './baseCommand.ts'; | ||
| export { Start, Start as StartCommand }; |
There was a problem hiding this comment.
Restore Stop command export for API parity.
The legacy @eggjs/scripts entrypoint exposed both StartCommand and StopCommand. By omitting Stop here we break existing programmatic consumers who require the stop command. Please re-export the class alongside Start so downstream integrations keep working.
-import Start from './commands/start.ts';
+import Start from './commands/start.ts';
+import Stop from './commands/stop.ts';
export * from './baseCommand.ts';
-export { Start, Start as StartCommand };
+export { Start, Start as StartCommand, Stop, Stop as StopCommand };📝 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.
| import Start from './commands/start.ts'; | |
| // exports.StopCommand = require('./lib/cmd/stop'); | |
| export * from './baseCommand.ts'; | |
| export { Start, Start as StartCommand }; | |
| import Start from './commands/start.ts'; | |
| import Stop from './commands/stop.ts'; | |
| // exports.StopCommand = require('./lib/cmd/stop'); | |
| export * from './baseCommand.ts'; | |
| export { Start, Start as StartCommand, Stop, Stop as StopCommand }; |
🤖 Prompt for AI Agents
In tools/scripts/src/index.ts around lines 1 to 6, the file only imports and
exports Start, but the legacy API also exported a Stop/StopCommand; restore
parity by importing the Stop command (e.g. import Stop from
'./commands/stop.ts') and re-exporting it alongside Start (export { Stop, Stop
as StopCommand }) so both Start and Stop/StopCommand are available to downstream
consumers.
| killer = coffee.fork(eggBin, ['stop', '--title=example-stop'], { cwd: fixturePath }) as Coffee; | ||
| killer.debug(); | ||
| // killer.expect('code', 0); | ||
| await killer.end(); | ||
|
|
There was a problem hiding this comment.
Reinstate exit-code assertions for stop helpers.
Both of these flows drop the .expect('code', 0) before awaiting killer.end(). If the CLI exits non-zero we would still pass, masking regressions in the stop command. Please keep the exit-code assertion active in each case.
killer = coffee.fork(eggBin, ['stop', '--title=example-stop'], { cwd: fixturePath }) as Coffee;
killer.debug();
- // killer.expect('code', 0);
+ killer.expect('code', 0);
await killer.end(); killer = coffee.fork(eggBin, ['stop'], { cwd: fixturePath }) as Coffee;
killer.debug();
- // killer.expect('code', 0);
+ killer.expect('code', 0);
await killer.end();Also applies to: 269-273
🤖 Prompt for AI Agents
In tools/scripts/test/stop.test.ts around lines 201-205 (and similarly for lines
269-273), the tests remove the .expect('code', 0) assertion before awaiting
killer.end(), which allows non-zero CLI exits to pass silently; restore the
exit-code assertion by re-adding .expect('code', 0) on the Coffee process chain
before awaiting killer.end() so the test fails when the stop command exits with
a non-zero code.
| import { mm, restore } from 'mm'; | ||
| import { detectPort } from 'detect-port'; | ||
|
|
||
| import { cleanup, type Coffee } from './utils.ts'; | ||
| import { isWindows } from '../src/helper.ts'; | ||
|
|
||
| const __dirname = import.meta.dirname; | ||
|
|
||
| describe('test/ts.test.ts', () => { | ||
| const eggBin = path.join(__dirname, '../bin/run.js'); | ||
| const homePath = path.join(__dirname, 'fixtures/home'); | ||
| const waitTime = 5000; | ||
| let fixturePath: string; | ||
|
|
||
| beforeEach(() => mm(process.env, 'MOCK_HOME_DIR', homePath)); | ||
| afterEach(restore); |
There was a problem hiding this comment.
Fix mm import: named import is undefined at runtime
mm only exposes a default export with .restore. Importing { mm, restore } leaves both undefined, so beforeEach throws and the suite never runs. Switch to the default import and call mm.restore() in afterEach.
-import { mm, restore } from 'mm';
+import mm from 'mm';
...
- afterEach(restore);
+ afterEach(() => mm.restore());📝 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.
| import { mm, restore } from 'mm'; | |
| import { detectPort } from 'detect-port'; | |
| import { cleanup, type Coffee } from './utils.ts'; | |
| import { isWindows } from '../src/helper.ts'; | |
| const __dirname = import.meta.dirname; | |
| describe('test/ts.test.ts', () => { | |
| const eggBin = path.join(__dirname, '../bin/run.js'); | |
| const homePath = path.join(__dirname, 'fixtures/home'); | |
| const waitTime = 5000; | |
| let fixturePath: string; | |
| beforeEach(() => mm(process.env, 'MOCK_HOME_DIR', homePath)); | |
| afterEach(restore); | |
| import mm from 'mm'; | |
| import { detectPort } from 'detect-port'; | |
| import { cleanup, type Coffee } from './utils.ts'; | |
| import { isWindows } from '../src/helper.ts'; | |
| const __dirname = import.meta.dirname; | |
| describe('test/ts.test.ts', () => { | |
| const eggBin = path.join(__dirname, '../bin/run.js'); | |
| const homePath = path.join(__dirname, 'fixtures/home'); | |
| const waitTime = 5000; | |
| let fixturePath: string; | |
| beforeEach(() => mm(process.env, 'MOCK_HOME_DIR', homePath)); | |
| afterEach(() => mm.restore()); |
🤖 Prompt for AI Agents
In tools/scripts/test/ts.test.ts around lines 9 to 24, the current import uses
named imports ({ mm, restore }) but mm is a default export; replace the named
imports with a single default import (e.g., import mm from 'mm') and update the
test teardown to call mm.restore() in afterEach instead of using a non-existent
restore binding; keep the existing beforeEach call to mm(...) as-is so it uses
the default export function.
| 'packages/*', | ||
| 'plugins/*', | ||
| 'tools/*', | ||
| 'tools/create-egg', |
There was a problem hiding this comment.
Restore test coverage for tools/scripts.
Switching from 'tools/*' to 'tools/create-egg' means the root pnpm run test (which runs vitest with this config) no longer executes the Vitest suites under tools/scripts. With the new CLI/tooling added in this PR, skipping those tests is a significant regression in coverage and increases the risk of shipping broken behavior. Please include the scripts workspace again (e.g., add 'tools/scripts' or revert to the glob) so its tests run as part of the main test command.
🤖 Prompt for AI Agents
In vitest.config.ts around line 8, the test include list was narrowed to
'tools/create-egg', which unintentionally omits the Vitest suites under
tools/scripts; restore coverage by adding the scripts workspace back (e.g., add
'tools/scripts' to the include array or revert the entry to the original glob
like 'tools/*') so that pnpm run test executes those tests as part of the main
suite.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci.yml(1 hunks)plugins/schedule/test/dynamic.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Prefer TypeScript and ESM: write sources and exports in .ts (ESM-first) rather than CommonJS
Use two-space indentation, trailing commas, and semicolons (Prettier/oxlint defaults)
Name files in lowercase with hyphens (e.g., loader-context.ts)
Name classes in PascalCase
Name functions and variables in camelCase
Re-export types thoughtfully to keep the public API stable
Files:
plugins/schedule/test/dynamic.test.ts
**/test/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/test/**/*.test.ts: Place test suites following Vitest discovery: /test//*.test.ts
Mirror the repository test pattern when adding new suitesTest files should be named with the pattern test/**/*.test.ts
Files:
plugins/schedule/test/dynamic.test.ts
plugins/**/test/**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Plugin tests must use Vitest (import from 'vitest') and follow the test file pattern test/**/*.test.ts
Files:
plugins/schedule/test/dynamic.test.ts
🧬 Code graph analysis (1)
plugins/schedule/test/dynamic.test.ts (1)
plugins/schedule/test/utils.ts (1)
contains(28-32)
⏰ 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). (21)
- GitHub Check: Test (macos-latest, 24, tools)
- GitHub Check: Test (windows-latest, 24, plugins)
- GitHub Check: Test (macos-latest, 22, tools)
- GitHub Check: Test (macos-latest, 22, plugins)
- GitHub Check: Test (windows-latest, 22, plugins)
- GitHub Check: Test (ubuntu-latest, 24, plugins)
- GitHub Check: Test (windows-latest, 22, tools)
- GitHub Check: Test (windows-latest, 22, packages)
- GitHub Check: Test (macos-latest, 22, packages)
- GitHub Check: Test (windows-latest, 24, packages)
- GitHub Check: Test (ubuntu-latest, 22, packages)
- GitHub Check: Test (macos-latest, 24, plugins)
- GitHub Check: Test (ubuntu-latest, 22, plugins)
- GitHub Check: Test (ubuntu-latest, 24, packages)
- GitHub Check: Test egg-scripts (ubuntu-latest, 22, 4/4)
- GitHub Check: Test egg-scripts (ubuntu-latest, 22, 1/4)
- GitHub Check: Test egg-bin (windows-latest, 22)
- GitHub Check: Test egg-bin (ubuntu-latest, 22)
- GitHub Check: typecheck
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Cloudflare Pages: egg-v3
🔇 Additional comments (1)
plugins/schedule/test/dynamic.test.ts (1)
23-23: Document rationale fortoBeGreaterThanOrEqual(1).
The change aligns with other timing-sensitive tests (e.g.all.test.ts,subscription.test.ts) that usetoBeGreaterThanOrEqualto avoid flakiness. Add a brief inline comment indynamic.test.tsexplaining that cron may fire multiple times in 5 s, so ≥ 1 is acceptable.
| - name: Run tests | ||
| run: pnpm run --filter="@eggjs/scripts" ci --reporter=blob --shard=${{ matrix.shardIndex }}/${{ matrix.shardTotal }} | ||
|
|
There was a problem hiding this comment.
Forward Vitest flags with -- separator.
pnpm run does not pass --reporter/--shard through unless you insert -- before them. As written, the shard flags are dropped, so each shard re-runs the entire suite and the blob reporter never activates—defeating the sharding/coverage objective. Update the step to pnpm run --filter="@eggjs/scripts" ci -- --reporter=blob --shard=....
🤖 Prompt for AI Agents
In .github/workflows/ci.yml around lines 214-216, the pnpm run invocation is not
forwarding Vitest flags because it lacks the `--` separator; update the step to
insert `--` after the script and before the vitest flags so the command forwards
`--reporter=blob` and `--shard=${{ matrix.shardIndex }}/${{ matrix.shardTotal
}}` to the underlying script (i.e., change to use pnpm run --filter="... " ci --
--reporter=blob --shard=...).
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
.github/workflows/ci.yml (1)
219-220: Still missing the--separator before Vitest flags.
pnpm rundrops--shard=...unless you insert--before the Vitest arguments, so each shard currently reruns the whole suite. Please add the separator so the CLI receives the shard flags.- run: pnpm run --filter="@eggjs/scripts" ci --shard=${{ matrix.shardIndex }}/${{ matrix.shardTotal }} + run: pnpm run --filter="@eggjs/scripts" ci -- --shard=${{ matrix.shardIndex }}/${{ matrix.shardTotal }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/workflows/ci.yml(2 hunks)plugins/security/src/app/extend/context.ts(1 hunks)tools/scripts/test/start-without-demon-1.test.ts(1 hunks)tools/scripts/test/start-without-demon-2.test.ts(1 hunks)tools/scripts/test/start-without-demon-3.test.ts(1 hunks)tools/scripts/test/start-without-demon-4.test.ts(1 hunks)tools/scripts/test/start-without-demon-5.test.ts(1 hunks)tools/scripts/vitest.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- plugins/security/src/app/extend/context.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Prefer TypeScript and ESM: write sources and exports in .ts (ESM-first) rather than CommonJS
Use two-space indentation, trailing commas, and semicolons (Prettier/oxlint defaults)
Name files in lowercase with hyphens (e.g., loader-context.ts)
Name classes in PascalCase
Name functions and variables in camelCase
Re-export types thoughtfully to keep the public API stable
Files:
tools/scripts/test/start-without-demon-5.test.tstools/scripts/test/start-without-demon-3.test.tstools/scripts/test/start-without-demon-4.test.tstools/scripts/test/start-without-demon-2.test.tstools/scripts/test/start-without-demon-1.test.tstools/scripts/vitest.config.ts
**/test/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/test/**/*.test.ts: Place test suites following Vitest discovery: /test//*.test.ts
Mirror the repository test pattern when adding new suitesTest files should be named with the pattern test/**/*.test.ts
Files:
tools/scripts/test/start-without-demon-5.test.tstools/scripts/test/start-without-demon-3.test.tstools/scripts/test/start-without-demon-4.test.tstools/scripts/test/start-without-demon-2.test.tstools/scripts/test/start-without-demon-1.test.ts
{packages,plugins,tools}/**/vitest.config.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Each Vitest-using package should include a vitest.config.ts
Files:
tools/scripts/vitest.config.ts
🧠 Learnings (6)
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to tools/egg-bin/test/**/*.test.ts : The egg-bin tool package must use Mocha for tests with files under test/**/*.test.ts
Applied to files:
tools/scripts/test/start-without-demon-5.test.tstools/scripts/test/start-without-demon-3.test.tstools/scripts/test/start-without-demon-1.test.ts
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to packages/{egg,core,utils,mock,cluster,koa,supertest,extend2,koa-static-cache,router}/test/**/*.test.ts : Core packages must use Vitest (import from 'vitest') with test files under test/**/*.test.ts
Applied to files:
tools/scripts/test/start-without-demon-5.test.tstools/scripts/test/start-without-demon-3.test.tstools/scripts/test/start-without-demon-2.test.tstools/scripts/test/start-without-demon-1.test.tstools/scripts/vitest.config.ts
📚 Learning: 2025-09-18T07:56:17.417Z
Learnt from: CR
PR: eggjs/egg#0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T07:56:17.417Z
Learning: For features affecting HTTP or process orchestration, add regression tests that exercise both the CommonJS and TypeScript example apps
Applied to files:
tools/scripts/test/start-without-demon-3.test.tstools/scripts/test/start-without-demon-2.test.tstools/scripts/test/start-without-demon-1.test.ts
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to {packages,plugins,tools}/**/vitest.config.ts : Each Vitest-using package should include a vitest.config.ts
Applied to files:
tools/scripts/vitest.config.ts
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to packages/**/test/**/*.test.ts : Use import { describe, it } from 'vitest' in tests
Applied to files:
tools/scripts/vitest.config.ts
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to plugins/**/test/**/*.test.ts : Plugin tests must use Vitest (import from 'vitest') and follow the test file pattern test/**/*.test.ts
Applied to files:
tools/scripts/vitest.config.ts
🧬 Code graph analysis (5)
tools/scripts/test/start-without-demon-5.test.ts (3)
packages/egg/test/utils.ts (2)
mm(23-23)restore(29-29)tools/scripts/test/utils.ts (3)
Coffee(8-8)cleanup(10-44)replaceWeakRefMessage(46-52)tools/scripts/src/helper.ts (1)
isWindows(3-3)
tools/scripts/test/start-without-demon-3.test.ts (2)
packages/egg/test/utils.ts (2)
mm(23-23)restore(29-29)tools/scripts/test/utils.ts (3)
Coffee(8-8)cleanup(10-44)replaceWeakRefMessage(46-52)
tools/scripts/test/start-without-demon-4.test.ts (1)
tools/scripts/test/utils.ts (3)
Coffee(8-8)cleanup(10-44)replaceWeakRefMessage(46-52)
tools/scripts/test/start-without-demon-2.test.ts (2)
packages/egg/test/utils.ts (2)
mm(23-23)restore(29-29)tools/scripts/test/utils.ts (3)
Coffee(8-8)cleanup(10-44)replaceWeakRefMessage(46-52)
tools/scripts/test/start-without-demon-1.test.ts (1)
tools/scripts/test/utils.ts (3)
Coffee(8-8)cleanup(10-44)replaceWeakRefMessage(46-52)
⏰ 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). (20)
- GitHub Check: Test (ubuntu-latest, 22, plugins)
- GitHub Check: Test (windows-latest, 24, packages)
- GitHub Check: Test (macos-latest, 22, packages)
- GitHub Check: Test (ubuntu-latest, 24, tools)
- GitHub Check: Test (windows-latest, 22, tools)
- GitHub Check: Test (macos-latest, 24, plugins)
- GitHub Check: Test (macos-latest, 24, packages)
- GitHub Check: Test (ubuntu-latest, 22, packages)
- GitHub Check: Test (ubuntu-latest, 24, packages)
- GitHub Check: Test egg-bin (windows-latest, 22, 2/4)
- GitHub Check: Test (macos-latest, 22, plugins)
- GitHub Check: Test egg-bin (windows-latest, 22, 1/4)
- GitHub Check: Test egg-bin (ubuntu-latest, 22, 1/4)
- GitHub Check: Test egg-bin (ubuntu-latest, 22, 2/4)
- GitHub Check: Test egg-bin (windows-latest, 22, 4/4)
- GitHub Check: Test egg-scripts (ubuntu-latest, 22, 4/4)
- GitHub Check: Test egg-scripts (ubuntu-latest, 22, 2/4)
- GitHub Check: Test egg-scripts (ubuntu-latest, 22, 3/4)
- GitHub Check: Test egg-scripts (ubuntu-latest, 22, 1/4)
- GitHub Check: Cloudflare Pages: egg-v3
🔇 Additional comments (1)
tools/scripts/vitest.config.ts (1)
1-13: LGTM! Configuration is appropriate for integration tests.The Vitest configuration is correct and follows monorepo standards. The 60-second timeouts are appropriate for integration tests that spawn child processes (start/stop commands), and the test file pattern matches the coding guidelines.
Based on learnings and coding guidelines.
| afterAll(async () => { | ||
| app.proc.kill('SIGTERM'); | ||
| await cleanup(fixturePath); | ||
| }); |
There was a problem hiding this comment.
Guard teardown when the child process never spawned
If coffee.fork throws before assigning app (e.g., spawn ENOENT), this afterAll crashes with “Cannot read properties of undefined (reading 'proc')”, hiding the real root cause and leaving the suite dirty. Please null-check before killing (and mirror the same fix for the other teardown blocks in this file).
afterAll(async () => {
- app.proc.kill('SIGTERM');
+ if (app?.proc) app.proc.kill('SIGTERM');
await cleanup(fixturePath);
});📝 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.
| afterAll(async () => { | |
| app.proc.kill('SIGTERM'); | |
| await cleanup(fixturePath); | |
| }); | |
| afterAll(async () => { | |
| if (app?.proc) app.proc.kill('SIGTERM'); | |
| await cleanup(fixturePath); | |
| }); |
🤖 Prompt for AI Agents
In tools/scripts/test/start-without-demon-1.test.ts around lines 40 to 43, the
afterAll teardown assumes app is defined and calls app.proc.kill, which throws
if coffee.fork failed and app was never assigned; modify the teardown to
null-check app (and app.proc) before attempting to kill the child and before
cleanup, e.g., only call app.proc.kill if app && app.proc exist, and apply the
same defensive null-check pattern to the other teardown blocks in this file so
the real spawn error isn't obscured and tests clean up safely.
| app.proc.kill('SIGTERM'); | ||
| await cleanup(fixturePath); | ||
| }); |
There was a problem hiding this comment.
Defensive teardown to avoid masking test failures
If the fork fails before app is set, this teardown throws while dereferencing app.proc. Add a guard so the cleanup still happens and the original error surfaces (apply across the file).
afterAll(async () => {
- app.proc.kill('SIGTERM');
+ if (app?.proc) app.proc.kill('SIGTERM');
await cleanup(fixturePath);
});📝 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.
| app.proc.kill('SIGTERM'); | |
| await cleanup(fixturePath); | |
| }); | |
| afterAll(async () => { | |
| if (app?.proc) app.proc.kill('SIGTERM'); | |
| await cleanup(fixturePath); | |
| }); |
🤖 Prompt for AI Agents
In tools/scripts/test/start-without-demon-2.test.ts around lines 40 to 42, the
teardown assumes `app` and `app.proc` exist and will throw if the fork failed
earlier; change the teardown to defensively check that `app` and `app.proc` are
defined before calling `app.proc.kill('SIGTERM')`, but always call `await
cleanup(fixturePath)` (use a guard or optional chaining and ensure cleanup runs
in a finally or after the guarded kill); apply the same pattern to other
teardowns in this file so failures during setup don't get masked by teardown
exceptions.
| afterAll(async () => { | ||
| app.proc.kill('SIGTERM'); | ||
| await cleanup(fixturePath); | ||
| }); |
There was a problem hiding this comment.
Harden teardown against failed spawns
When coffee.fork fails before assigning app, this afterAll throws while trying to access app.proc, obscuring the real failure. Please wrap the kill call with an existence check (and apply the same defensive guard to every teardown in this suite).
afterAll(async () => {
- app.proc.kill('SIGTERM');
+ if (app?.proc) app.proc.kill('SIGTERM');
await cleanup(fixturePath);
});📝 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.
| afterAll(async () => { | |
| app.proc.kill('SIGTERM'); | |
| await cleanup(fixturePath); | |
| }); | |
| afterAll(async () => { | |
| if (app?.proc) app.proc.kill('SIGTERM'); | |
| await cleanup(fixturePath); | |
| }); |
🤖 Prompt for AI Agents
In tools/scripts/test/start-without-demon-5.test.ts around lines 42 to 45, the
afterAll teardown assumes app and app.proc exist and will throw if coffee.fork
failed; update the teardown to check that app is defined and app.proc is defined
before calling app.proc.kill('SIGTERM'), and do the same defensive existence
check for every teardown in this test suite (wrap any app.proc access in a guard
or conditional to skip killing/cleanup when app was never created).
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/redis/package.json (1)
72-72: Use catalog: for external devDependency to follow workspace policy.detect-port should reference the catalog version, not a caret range. As per coding guidelines.
- "detect-port": "^2.1.0", + "detect-port": "catalog:",
♻️ Duplicate comments (2)
.github/workflows/ci.yml (2)
134-134: Insert--to forward Vitest shard flags.Without
--, pnpm does not pass--shardto the script; each shard will run the full suite. Fix:- run: pnpm run ci --shard=${{ matrix.shardIndex }}/${{ matrix.shardTotal }} + run: pnpm run ci -- --shard=${{ matrix.shardIndex }}/${{ matrix.shardTotal }}
224-225: Forward flags when running filtered package script.Add
--so vitest receives--shard. Otherwise sharding is ignored.- run: pnpm run --filter="@eggjs/scripts" ci --shard=${{ matrix.shardIndex }}/${{ matrix.shardTotal }} + run: pnpm run --filter="@eggjs/scripts" ci -- --shard=${{ matrix.shardIndex }}/${{ matrix.shardTotal }}
🧹 Nitpick comments (3)
plugins/redis/package.json (1)
56-56: Build script LGTM.Inline clean + build is fine. If Windows shells are a concern, consider quoting the glob or using rimraf’s --glob flag, but not required.
.github/workflows/ci.yml (1)
54-69: Consider job-level timeouts to avoid hung runners.Add
timeout-minutes(e.g., 30–60) to shard jobs to auto-cancel on hangs (Windows Redis, coverage upload, network). Keeps CI capacity healthy.Also applies to: 143-160, 191-207
CLAUDE.md (1)
418-423: Add missingcleanscript and clarify build chain intent
- Key points say all plugins must have build, clean, prepublishOnly, but the snippet lacks
clean. Add it. As per learnings.- Verify whether running tsdown before tsc is intentional (tsdown for unused checks, tsc for final output). If not, consider simplifying to tsdown-only to avoid double compilation.
Minimal fix to include
cleanand simplify build (Option A). If you need tsc as final step (Option B), keep both but clean first:"scripts": { - "build": "tsdown && rimraf dist *.tsbuildinfo && tsc -p tsconfig.build.json", + // Option A (simpler, tsdown-only build): + "build": "rimraf dist *.tsbuildinfo && tsdown", + // Option B (tsdown checks, tsc final output): + // "build": "rimraf dist *.tsbuildinfo && tsdown && tsc -p tsconfig.build.json", + "clean": "rimraf dist *.tsbuildinfo", "typecheck": "tsc --noEmit", "lint": "oxlint --type-aware", "test": "vitest run", "prepublishOnly": "pnpm run build" }
📜 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 (4)
.github/workflows/ci.yml(4 hunks)CLAUDE.md(2 hunks)plugins/redis/package.json(1 hunks)plugins/typebox-validate/package.json(0 hunks)
💤 Files with no reviewable changes (1)
- plugins/typebox-validate/package.json
🧰 Additional context used
📓 Path-based instructions (3)
{package.json,{packages,plugins,tools}/**/package.json}
📄 CodeRabbit inference engine (CLAUDE.md)
{package.json,{packages,plugins,tools}/**/package.json}: All packages must require Node.js >= 22.18.0 (set engines.node accordingly)
Use workspace:* for internal dependencies and catalog: for external dependencies defined in pnpm-workspace.yaml
Files:
plugins/redis/package.json
{packages,plugins,tools}/**/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
Each package must define scripts: typecheck as "tsc --noEmit" and lint as "oxlint --type-aware"
Files:
plugins/redis/package.json
plugins/**/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
plugins/**/package.json: All plugins must include egg as a peerDependency with version "workspace:*"
Plugins' package.json exports must point to TS sources during development and mirror to dist via publishConfig.exports
Files:
plugins/redis/package.json
🧠 Learnings (4)
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to {packages,plugins,tools}/**/package.json : Each package must define scripts: typecheck as "tsc --noEmit" and lint as "oxlint --type-aware"
Applied to files:
CLAUDE.mdplugins/redis/package.json
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to {packages,plugins,tools}/**/tsdown.config.ts : Each package should use tsdown for builds and include a tsdown.config.ts
Applied to files:
CLAUDE.md
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to {packages,plugins,tools}/**/vitest.config.ts : Each Vitest-using package should include a vitest.config.ts
Applied to files:
CLAUDE.md
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to packages/**/test/**/*.test.ts : Name test files as test/**/*.test.ts and run them with Vitest
Applied to files:
CLAUDE.md
⏰ 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). (20)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: Test (ubuntu-latest, 24, 2/4)
- GitHub Check: Test (ubuntu-latest, 22, 1/4)
- GitHub Check: Test (windows-latest, 22, 1/4)
- GitHub Check: Test (ubuntu-latest, 24, 4/4)
- GitHub Check: Test (windows-latest, 22, 2/4)
- GitHub Check: Test (ubuntu-latest, 22, 2/4)
- GitHub Check: Test (ubuntu-latest, 22, 4/4)
- GitHub Check: Test (ubuntu-latest, 24, 1/4)
- GitHub Check: Test (ubuntu-latest, 22, 3/4)
- GitHub Check: Test egg-bin (windows-latest, 22, 0/4)
- GitHub Check: Test egg-bin (ubuntu-latest, 22, 2/4)
- GitHub Check: Test egg-bin (ubuntu-latest, 22, 0/4)
- GitHub Check: Test egg-bin (windows-latest, 22, 3/4)
- GitHub Check: Test egg-bin (windows-latest, 22, 2/4)
- GitHub Check: Test egg-scripts (ubuntu-latest, 22, 4/4)
- GitHub Check: Test egg-scripts (ubuntu-latest, 22, 3/4)
- GitHub Check: Test egg-scripts (ubuntu-latest, 22, 1/4)
- GitHub Check: Test egg-scripts (ubuntu-latest, 22, 2/4)
- GitHub Check: typecheck
🔇 Additional comments (2)
.github/workflows/ci.yml (1)
60-62: Confirm shard indexing conventions.You’re using 1-based shards for Vitest and 0-based CI_NODE_* for egg-bin. That matches Vitest’s 1-based
--shard i/nsemantics and common CI env conventions. Please confirm this is intentional across scripts/configs.Also applies to: 151-152
CLAUDE.md (1)
303-316: Enablingunbundle: truein the tsdown template looks goodThis aligns with the unbundled ESM output strategy and unused checks.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/koa-static-cache/test/index.test.ts (1)
261-275: Consider removing or fixing the skipped duplicate test.This test is marked as
.skipand appears to duplicate the logic of the previous test (lines 245-259). Consider either removing it if it's no longer needed, or un-skipping it with a clear distinction from the first test if it's meant to test a different scenario.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
package.json(1 hunks)packages/koa-static-cache/test/index.test.ts(2 hunks)tools/egg-bin/test/commands/dev.test.ts(1 hunks)tools/egg-bin/test/commands/dev/commonjs-app.test.ts(1 hunks)tools/egg-bin/test/commands/dev/detect-port.test.ts(1 hunks)tools/egg-bin/test/commands/dev/esm-app.ts.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
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
Files:
packages/koa-static-cache/test/index.test.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Prefer TypeScript and ESM: write sources and exports in .ts (ESM-first) rather than CommonJS
Use two-space indentation, trailing commas, and semicolons (Prettier/oxlint defaults)
Name files in lowercase with hyphens (e.g., loader-context.ts)
Name classes in PascalCase
Name functions and variables in camelCase
Re-export types thoughtfully to keep the public API stable
Files:
packages/koa-static-cache/test/index.test.tstools/egg-bin/test/commands/dev/detect-port.test.tstools/egg-bin/test/commands/dev/esm-app.ts.tstools/egg-bin/test/commands/dev/commonjs-app.test.tstools/egg-bin/test/commands/dev.test.ts
**/test/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/test/**/*.test.ts: Place test suites following Vitest discovery: /test//*.test.ts
Mirror the repository test pattern when adding new suitesTest files should be named with the pattern test/**/*.test.ts
Files:
packages/koa-static-cache/test/index.test.tstools/egg-bin/test/commands/dev/detect-port.test.tstools/egg-bin/test/commands/dev/commonjs-app.test.tstools/egg-bin/test/commands/dev.test.ts
packages/{egg,core,utils,mock,cluster,koa,supertest,extend2,koa-static-cache,router}/test/**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Core packages must use Vitest (import from 'vitest') with test files under test/**/*.test.ts
Files:
packages/koa-static-cache/test/index.test.ts
tools/egg-bin/test/**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
The egg-bin tool package must use Mocha for tests with files under test/**/*.test.ts
Files:
tools/egg-bin/test/commands/dev/detect-port.test.tstools/egg-bin/test/commands/dev/commonjs-app.test.tstools/egg-bin/test/commands/dev.test.ts
{package.json,{packages,plugins,tools}/**/package.json}
📄 CodeRabbit inference engine (CLAUDE.md)
{package.json,{packages,plugins,tools}/**/package.json}: All packages must require Node.js >= 22.18.0 (set engines.node accordingly)
Use workspace:* for internal dependencies and catalog: for external dependencies defined in pnpm-workspace.yaml
Files:
package.json
🧠 Learnings (3)
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to tools/egg-bin/test/**/*.test.ts : The egg-bin tool package must use Mocha for tests with files under test/**/*.test.ts
Applied to files:
tools/egg-bin/test/commands/dev/esm-app.ts.tstools/egg-bin/test/commands/dev/commonjs-app.test.tstools/egg-bin/test/commands/dev.test.ts
📚 Learning: 2025-09-18T07:56:17.417Z
Learnt from: CR
PR: eggjs/egg#0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T07:56:17.417Z
Learning: For features affecting HTTP or process orchestration, add regression tests that exercise both the CommonJS and TypeScript example apps
Applied to files:
tools/egg-bin/test/commands/dev/esm-app.ts.tstools/egg-bin/test/commands/dev/commonjs-app.test.ts
📚 Learning: 2025-10-05T17:37:55.210Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-05T17:37:55.210Z
Learning: Applies to packages/{egg,core,utils,mock,cluster,koa,supertest,extend2,koa-static-cache,router}/test/**/*.test.ts : Core packages must use Vitest (import from 'vitest') with test files under test/**/*.test.ts
Applied to files:
tools/egg-bin/test/commands/dev/esm-app.ts.tstools/egg-bin/test/commands/dev/commonjs-app.test.tstools/egg-bin/test/commands/dev.test.ts
🧬 Code graph analysis (1)
tools/egg-bin/test/commands/dev/detect-port.test.ts (1)
examples/helloworld-commonjs/index.js (1)
server(13-13)
⏰ 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). (19)
- GitHub Check: Test (ubuntu-latest, 24, 4/4)
- GitHub Check: Test (macos-latest, 22, 2/4)
- GitHub Check: Test (ubuntu-latest, 22, 3/4)
- GitHub Check: Test (windows-latest, 24, 3/4)
- GitHub Check: Test (macos-latest, 24, 4/4)
- GitHub Check: Test (ubuntu-latest, 24, 2/4)
- GitHub Check: Test (windows-latest, 22, 3/4)
- GitHub Check: Test (windows-latest, 22, 1/4)
- GitHub Check: Test (ubuntu-latest, 24, 3/4)
- GitHub Check: Test egg-bin (windows-latest, 22, 3/4)
- GitHub Check: Test egg-bin (ubuntu-latest, 22, 1/4)
- GitHub Check: Test egg-bin (ubuntu-latest, 22, 2/4)
- GitHub Check: Test egg-bin (windows-latest, 22, 2/4)
- GitHub Check: Test egg-bin (ubuntu-latest, 22, 0/4)
- GitHub Check: Test egg-scripts (ubuntu-latest, 22, 2/4)
- GitHub Check: Test egg-scripts (ubuntu-latest, 22, 3/4)
- GitHub Check: Test egg-scripts (ubuntu-latest, 22, 4/4)
- GitHub Check: typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
packages/koa-static-cache/test/index.test.ts (1)
257-257: LGTM! Timing adjustment improves test reliability.The increased wait time from 10ms to 1000ms provides better stability for file system operations across different CI/CD environments, reducing potential flakiness in Last-Modified header tests.
1 participant
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests
Chores