Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/workflows/e2e-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,10 @@ jobs:
npm run lint
npm run test
npm run prepublishOnly

# Manifest E2E: generate, validate, boot with manifest, clean
node ../../ecosystem-ci/scripts/verify-manifest.mjs
cd ..
steps:
- uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6
- uses: ./.github/actions/clone
Expand Down
128 changes: 128 additions & 0 deletions ecosystem-ci/scripts/verify-manifest.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
#!/usr/bin/env node

/**
* E2E verification script for the egg-bin manifest CLI.
*
* Run this inside a project directory that has egg-bin and egg installed.
* It tests the full manifest lifecycle:
* 1. generate — creates .egg/manifest.json via metadataOnly boot
* 2. validate — verifies the manifest is structurally valid
* 3. boot with manifest — starts the app using the manifest and health-checks it
* 4. clean — removes .egg/manifest.json
*/

import { execSync } from 'node:child_process';
import { existsSync, readFileSync, rmSync } from 'node:fs';
import { join } from 'node:path';

const projectDir = process.cwd();
const manifestPath = join(projectDir, '.egg', 'manifest.json');
const env = process.env.MANIFEST_VERIFY_ENV || 'unittest';
const healthPort = process.env.MANIFEST_VERIFY_PORT || '7002';
const healthTimeout = parseInt(process.env.MANIFEST_VERIFY_TIMEOUT || '30', 10);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The parseInt function can return NaN if process.env.MANIFEST_VERIFY_TIMEOUT is not a valid number. Using NaN in subsequent calculations or comparisons can lead to unexpected behavior. It's safer to check for isNaN or provide a default fallback if parsing fails. Note: ensure the variable is declared with let to allow reassignment.

let healthTimeout = parseInt(process.env.MANIFEST_VERIFY_TIMEOUT || '30', 10);
if (isNaN(healthTimeout)) {
  console.warn('Invalid MANIFEST_VERIFY_TIMEOUT, defaulting to 30s');
  healthTimeout = 30;
}


function run(cmd) {
console.log(`\n$ ${cmd}`);
execSync(cmd, { stdio: 'inherit', cwd: projectDir });

Check warning

Code scanning / CodeQL

Indirect uncontrolled command line Medium

This command depends on an unsanitized
environment variable
.
This command depends on an unsanitized
environment variable
.

Copilot Autofix

AI 2 days ago

In general, the right fix is to avoid passing a single concatenated command string to execSync, and instead use an API that does not invoke a shell and takes the executable plus its arguments as an array, e.g. execFileSync. This inherently avoids shell parsing, so environment variables like MANIFEST_VERIFY_ENV and MANIFEST_VERIFY_PORT are passed as literal arguments and cannot alter the shell command structure.

For this file, the simplest change without altering functionality is:

  • Replace the generic run(cmd) and runCapture(cmd) helpers with argument-based wrappers that:
    • Accept command and args separately.
    • Log a reconstructed “pretty” shell-style command for human readability (using proper quoting when desired).
    • Call execFileSync (or execSync with { shell: false }, but execFileSync is clearer) with the command and argument array.
  • Update the call site that currently passes a string built with template interpolation:
    • Replace run(\npx egg-bin manifest generate --env=${env}`);`
    • With something like run('npx', ['egg-bin', 'manifest', 'generate', --env=${env}]);
  • Optionally, introduce a small helper to format the logged command (formatCommandForLog) to mimic shell syntax in logs, but that helper should not actually invoke a shell—only build a string for console.log.

All changes are confined to ecosystem-ci/scripts/verify-manifest.mjs. We can reuse the existing execSync import by switching to execFileSync (also exported from node:child_process), which requires a small import change. No change in observable behavior occurs other than removing the possibility of shell interpretation.

Suggested changeset 1
ecosystem-ci/scripts/verify-manifest.mjs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/ecosystem-ci/scripts/verify-manifest.mjs b/ecosystem-ci/scripts/verify-manifest.mjs
--- a/ecosystem-ci/scripts/verify-manifest.mjs
+++ b/ecosystem-ci/scripts/verify-manifest.mjs
@@ -11,7 +11,7 @@
  *   4. clean — removes .egg/manifest.json
  */
 
-import { execSync } from 'node:child_process';
+import { execFileSync } from 'node:child_process';
 import { existsSync, readFileSync, rmSync } from 'node:fs';
 import { join } from 'node:path';
 import { setTimeout as sleep } from 'node:timers/promises';
@@ -22,16 +22,21 @@
 const healthPort = process.env.MANIFEST_VERIFY_PORT || '7002';
 const healthTimeout = parseInt(process.env.MANIFEST_VERIFY_TIMEOUT || '60', 10);
 
-function run(cmd) {
-  console.log(`\n$ ${cmd}`);
-  execSync(cmd, { stdio: 'inherit', cwd: projectDir });
+function formatCommandForLog(command, args) {
+  const renderedArgs = (args || []).map(a => JSON.stringify(String(a)));
+  return [command].concat(renderedArgs).join(' ');
 }
 
-function runCapture(cmd) {
-  console.log(`\n$ ${cmd}`);
-  return execSync(cmd, { cwd: projectDir, encoding: 'utf-8' });
+function run(command, args = []) {
+  console.log(`\n$ ${formatCommandForLog(command, args)}`);
+  execFileSync(command, args, { stdio: 'inherit', cwd: projectDir });
 }
 
+function runCapture(command, args = []) {
+  console.log(`\n$ ${formatCommandForLog(command, args)}`);
+  return execFileSync(command, args, { cwd: projectDir, encoding: 'utf-8' });
+}
+
 function assert(condition, message) {
   if (!condition) {
     console.error(`FAIL: ${message}`);
@@ -52,7 +53,7 @@
 
 // Step 2: Generate manifest
 console.log('\n--- Step 1: Generate manifest ---');
-run(`npx egg-bin manifest generate --env=${env}`);
+run('npx', ['egg-bin', 'manifest', 'generate', `--env=${env}`]);
 
 // Step 3: Verify manifest file exists and has valid structure
 console.log('\n--- Step 2: Verify manifest structure ---');
EOF
@@ -11,7 +11,7 @@
* 4. clean removes .egg/manifest.json
*/

import { execSync } from 'node:child_process';
import { execFileSync } from 'node:child_process';
import { existsSync, readFileSync, rmSync } from 'node:fs';
import { join } from 'node:path';
import { setTimeout as sleep } from 'node:timers/promises';
@@ -22,16 +22,21 @@
const healthPort = process.env.MANIFEST_VERIFY_PORT || '7002';
const healthTimeout = parseInt(process.env.MANIFEST_VERIFY_TIMEOUT || '60', 10);

function run(cmd) {
console.log(`\n$ ${cmd}`);
execSync(cmd, { stdio: 'inherit', cwd: projectDir });
function formatCommandForLog(command, args) {
const renderedArgs = (args || []).map(a => JSON.stringify(String(a)));
return [command].concat(renderedArgs).join(' ');
}

function runCapture(cmd) {
console.log(`\n$ ${cmd}`);
return execSync(cmd, { cwd: projectDir, encoding: 'utf-8' });
function run(command, args = []) {
console.log(`\n$ ${formatCommandForLog(command, args)}`);
execFileSync(command, args, { stdio: 'inherit', cwd: projectDir });
}

function runCapture(command, args = []) {
console.log(`\n$ ${formatCommandForLog(command, args)}`);
return execFileSync(command, args, { cwd: projectDir, encoding: 'utf-8' });
}

function assert(condition, message) {
if (!condition) {
console.error(`FAIL: ${message}`);
@@ -52,7 +53,7 @@

// Step 2: Generate manifest
console.log('\n--- Step 1: Generate manifest ---');
run(`npx egg-bin manifest generate --env=${env}`);
run('npx', ['egg-bin', 'manifest', 'generate', `--env=${env}`]);

// Step 3: Verify manifest file exists and has valid structure
console.log('\n--- Step 2: Verify manifest structure ---');
Copilot is powered by AI and may make mistakes. Always verify output.
}

function runCapture(cmd) {
console.log(`\n$ ${cmd}`);
return execSync(cmd, { cwd: projectDir, encoding: 'utf-8' });

Check warning

Code scanning / CodeQL

Indirect uncontrolled command line Medium

This command depends on an unsanitized
environment variable
.

Copilot Autofix

AI 2 days ago

General fix: avoid executing tainted data via a shell. Prefer execFileSync / spawnSync with an argument array, and validate any environment-derived values (like ports) before using them. Where shell commands must remain string-based, perform strict validation/whitelisting.

Best minimal fix here:

  1. Keep the existing run/runCapture APIs for the rest of the file to avoid behavior changes.
  2. For the health-check curl command, stop going through the generic runCapture wrapper that uses execSync with a shell string.
  3. Instead, import execFileSync from node:child_process and call it directly with a fixed executable (curl) and an explicit argument array, passing healthUrl as a plain argument so it is not interpreted by the shell.
  4. Additionally, validate healthPort when it is read from the environment to ensure it is a reasonable numeric TCP port, falling back to a safe default (7002) if invalid. This removes the ability to inject shell metacharacters via the port value and also improves robustness.

Concretely:

  • Update the import on line 14 to also import execFileSync.
  • After healthTimeout is defined, parse and validate healthPort into a numeric healthPortNumber within 1–65535, then derive healthPort from that numeric value (ensuring it contains only digits).
  • Replace the runCapture call at line 91 with a direct execFileSync('curl', [...]) call that returns the HTTP status code without invoking a shell.

All changes are confined to ecosystem-ci/scripts/verify-manifest.mjs.

Suggested changeset 1
ecosystem-ci/scripts/verify-manifest.mjs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/ecosystem-ci/scripts/verify-manifest.mjs b/ecosystem-ci/scripts/verify-manifest.mjs
--- a/ecosystem-ci/scripts/verify-manifest.mjs
+++ b/ecosystem-ci/scripts/verify-manifest.mjs
@@ -11,7 +11,7 @@
  *   4. clean — removes .egg/manifest.json
  */
 
-import { execSync } from 'node:child_process';
+import { execSync, execFileSync } from 'node:child_process';
 import { existsSync, readFileSync, rmSync } from 'node:fs';
 import { join } from 'node:path';
 import { setTimeout as sleep } from 'node:timers/promises';
@@ -19,7 +19,15 @@
 const projectDir = process.cwd();
 const manifestPath = join(projectDir, '.egg', 'manifest.json');
 const env = process.env.MANIFEST_VERIFY_ENV || 'unittest';
-const healthPort = process.env.MANIFEST_VERIFY_PORT || '7002';
+const rawHealthPort = process.env.MANIFEST_VERIFY_PORT || '7002';
+const healthPortNumber = (() => {
+  const n = Number(rawHealthPort);
+  if (!Number.isInteger(n) || n < 1 || n > 65535) {
+    return 7002;
+  }
+  return n;
+})();
+const healthPort = String(healthPortNumber);
 const healthTimeout = parseInt(process.env.MANIFEST_VERIFY_TIMEOUT || '60', 10);
 
 function run(cmd) {
@@ -88,7 +96,7 @@
   console.log(`Waiting for app at ${healthUrl} (timeout: ${healthTimeout}s)...`);
   while (true) {
     try {
-      const output = runCapture(`curl -s -o /dev/null -w "%{http_code}" "${healthUrl}"`);
+      const output = execFileSync('curl', ['-s', '-o', '/dev/null', '-w', '%{http_code}', healthUrl], { cwd: projectDir, encoding: 'utf-8' });
       const status = output.trim();
       console.log('  Health check: status=%s', status);
       // Any HTTP response (not connection refused) means the app is up.
EOF
@@ -11,7 +11,7 @@
* 4. clean removes .egg/manifest.json
*/

import { execSync } from 'node:child_process';
import { execSync, execFileSync } from 'node:child_process';
import { existsSync, readFileSync, rmSync } from 'node:fs';
import { join } from 'node:path';
import { setTimeout as sleep } from 'node:timers/promises';
@@ -19,7 +19,15 @@
const projectDir = process.cwd();
const manifestPath = join(projectDir, '.egg', 'manifest.json');
const env = process.env.MANIFEST_VERIFY_ENV || 'unittest';
const healthPort = process.env.MANIFEST_VERIFY_PORT || '7002';
const rawHealthPort = process.env.MANIFEST_VERIFY_PORT || '7002';
const healthPortNumber = (() => {
const n = Number(rawHealthPort);
if (!Number.isInteger(n) || n < 1 || n > 65535) {
return 7002;
}
return n;
})();
const healthPort = String(healthPortNumber);
const healthTimeout = parseInt(process.env.MANIFEST_VERIFY_TIMEOUT || '60', 10);

function run(cmd) {
@@ -88,7 +96,7 @@
console.log(`Waiting for app at ${healthUrl} (timeout: ${healthTimeout}s)...`);
while (true) {
try {
const output = runCapture(`curl -s -o /dev/null -w "%{http_code}" "${healthUrl}"`);
const output = execFileSync('curl', ['-s', '-o', '/dev/null', '-w', '%{http_code}', healthUrl], { cwd: projectDir, encoding: 'utf-8' });
const status = output.trim();
console.log(' Health check: status=%s', status);
// Any HTTP response (not connection refused) means the app is up.
Copilot is powered by AI and may make mistakes. Always verify output.
}

function assert(condition, message) {
if (!condition) {
console.error(`FAIL: ${message}`);
process.exit(1);
}
console.log(`PASS: ${message}`);
}

console.log('=== Manifest E2E Verification ===');
console.log('Project: %s', projectDir);
console.log('Env: %s', env);

// Step 1: Clean any pre-existing manifest
if (existsSync(manifestPath)) {
rmSync(manifestPath);
console.log('Cleaned pre-existing manifest');
}

// Step 2: Generate manifest
console.log('\n--- Step 1: Generate manifest ---');
run(`npx egg-bin manifest generate --env=${env}`);

// Step 3: Verify manifest file exists and has valid structure
console.log('\n--- Step 2: Verify manifest structure ---');
assert(existsSync(manifestPath), '.egg/manifest.json exists after generate');

const manifest = JSON.parse(readFileSync(manifestPath, 'utf-8'));
assert(manifest.version === 1, 'manifest version is 1');
assert(typeof manifest.generatedAt === 'string' && manifest.generatedAt.length > 0, 'manifest has generatedAt');
assert(typeof manifest.invalidation === 'object', 'manifest has invalidation');
assert(manifest.invalidation.serverEnv === env, `manifest serverEnv matches "${env}"`);
assert(typeof manifest.resolveCache === 'object', 'manifest has resolveCache');
assert(typeof manifest.fileDiscovery === 'object', 'manifest has fileDiscovery');
assert(typeof manifest.extensions === 'object', 'manifest has extensions');

const resolveCacheCount = Object.keys(manifest.resolveCache).length;
const fileDiscoveryCount = Object.keys(manifest.fileDiscovery).length;
console.log(' resolveCache: %d entries', resolveCacheCount);
console.log(' fileDiscovery: %d entries', fileDiscoveryCount);

// Step 4: Validate manifest via CLI
console.log('\n--- Step 3: Validate manifest via CLI ---');
run(`npx egg-bin manifest validate --env=${env}`);

// Step 5: Boot the app with manifest and verify it starts correctly
console.log('\n--- Step 4: Boot app with manifest ---');
try {
run(`npx eggctl start --port=${healthPort} --env=${env} --daemon`);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Using eggctl start --daemon followed by a generic eggctl stop might not reliably stop the specific process started by this script, especially if multiple eggctl instances are running. This could lead to orphaned processes or interfere with subsequent test runs. Consider capturing the PID of the daemonized process and using it for a targeted kill command or ensuring eggctl stop has a mechanism to stop the specific instance started by the script.


const healthUrl = `http://127.0.0.1:${healthPort}/`;
const startTime = Date.now();
let ready = false;

console.log(`Waiting for app at ${healthUrl} (timeout: ${healthTimeout}s)...`);
while (true) {
try {
const output = runCapture(`curl -s -o /dev/null -w "%{http_code}" "${healthUrl}"`);
const status = output.trim();
console.log(' Health check: status=%s', status);
if (status === '200') {
ready = true;
break;
}
} catch {
console.log(' Health check: connection refused, retrying...');
}
Comment on lines +100 to +102
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The broad catch block (catch { ... }) for the curl command silently ignores any error that might occur during the health check, not just connection refused. This can hide other issues that prevent the app from starting correctly. It's better to log the error object (err) to provide more context for debugging.

Suggested change
} catch {
console.log(' Health check: connection refused, retrying...');
}
} catch (err) {
console.log(' Health check: connection refused, retrying... Error: %s', err.message);
}


const elapsed = (Date.now() - startTime) / 1000;
if (elapsed >= healthTimeout) {
console.log(' Health check timed out after %ds', elapsed);
break;
}

execSync('sleep 2');
}
Comment on lines +107 to +111
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The retry loop uses execSync('sleep 2'), which relies on an external shell command and will fail on Windows/non-POSIX environments. Consider replacing this with an in-process delay (e.g., await new Promise(r => setTimeout(r, 2000))) so the script is portable and doesn’t depend on sleep being available.

Copilot uses AI. Check for mistakes.

run(`npx eggctl stop`);
assert(ready, 'App booted successfully with manifest');
} catch (err) {
// Try to stop if started
try {
run(`npx eggctl stop`);
} catch {
/* ignore */
}
Comment on lines +119 to +121
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The catch block for eggctl stop during error handling is too broad and silently ignores any error. If eggctl stop fails for a reason other than the app not being started, this error will be hidden, making debugging difficult. Consider logging the error object (err) to provide more context.

Suggested change
} catch {
/* ignore */
}
} catch (err) {
console.error('Failed to stop eggctl during cleanup:', err.message);
}

console.error('Boot test failed:', err.message);
process.exit(1);
}

// Step 6: Clean manifest
console.log('\n--- Step 5: Clean manifest ---');
run(`npx egg-bin manifest clean`);
assert(!existsSync(manifestPath), '.egg/manifest.json removed after clean');

console.log('\n=== All manifest E2E checks passed! ===');
3 changes: 3 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions tools/egg-bin/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
"./baseCommand": "./src/baseCommand.ts",
"./commands/cov": "./src/commands/cov.ts",
"./commands/dev": "./src/commands/dev.ts",
"./commands/manifest": "./src/commands/manifest.ts",
"./commands/test": "./src/commands/test.ts",
"./types": "./src/types.ts",
"./utils": "./src/utils.ts",
Expand All @@ -42,6 +43,7 @@
"./baseCommand": "./dist/baseCommand.js",
"./commands/cov": "./dist/commands/cov.js",
"./commands/dev": "./dist/commands/dev.js",
"./commands/manifest": "./dist/commands/manifest.js",
"./commands/test": "./dist/commands/test.js",
"./types": "./dist/types.js",
"./utils": "./dist/utils.js",
Expand All @@ -56,6 +58,7 @@
"ci": "npm run cov"
},
"dependencies": {
"@eggjs/core": "workspace:*",
"@eggjs/tegg-vitest": "workspace:*",
"@eggjs/utils": "workspace:*",
"@oclif/core": "catalog:",
Expand Down
54 changes: 54 additions & 0 deletions tools/egg-bin/scripts/manifest-generate.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { debuglog } from 'node:util';

import { importModule } from '@eggjs/utils';

const debug = debuglog('egg/bin/scripts/manifest-generate');

async function main() {
debug('argv: %o', process.argv);
const options = JSON.parse(process.argv[2]);
debug('manifest generate options: %o', options);

// Set server env before importing framework
if (options.env) {
process.env.EGG_SERVER_ENV = options.env;
}

const framework = await importModule(options.framework);
const app = await framework.start({
baseDir: options.baseDir,
framework: options.framework,
env: options.env,
metadataOnly: true,
});
Comment on lines +27 to +33
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manifest generation boots the app in metadataOnly mode, but if a valid .egg/manifest.json already exists the loader will load that manifest and serve many lookups from cache, so the collector used by generateManifest() will only contain cache misses. That can cause the newly written manifest to be incomplete/delta-only. Consider removing/ignoring any existing manifest before framework.start(...) (e.g., ManifestStore.clean(baseDir) or unlinking the file) so generation always starts from a collector store.

Copilot uses AI. Check for mistakes.

// Generate manifest from collected metadata
const manifest = app.loader.generateManifest();

// Write manifest to .egg/manifest.json
// Resolve @eggjs/core from the framework path (not the project root),
// because pnpm strict mode may not hoist it to the project's node_modules.
const { ManifestStore } = await importModule('@eggjs/core', {
paths: [options.framework],
});
await ManifestStore.write(options.baseDir, manifest);

// Log stats
const resolveCacheCount = Object.keys(manifest.resolveCache).length;
const fileDiscoveryCount = Object.keys(manifest.fileDiscovery).length;
const extensionCount = Object.keys(manifest.extensions).length;
console.log('[manifest] Generated manifest v%d at %s', manifest.version, manifest.generatedAt);
console.log('[manifest] resolveCache entries: %d', resolveCacheCount);
console.log('[manifest] fileDiscovery entries: %d', fileDiscoveryCount);
console.log('[manifest] extension entries: %d', extensionCount);
console.log('[manifest] Written to %s/.egg/manifest.json', options.baseDir);

// Clean up and exit
await app.close();
process.exit(0);
}

main().catch((err) => {
console.error('[manifest] Generation failed:', err);
process.exit(1);
});
16 changes: 16 additions & 0 deletions tools/egg-bin/src/baseCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,22 @@ export abstract class BaseCommand<T extends typeof Command> extends Command {
}
}

protected async buildRequiresExecArgv(): Promise<string[]> {
const requires = await this.formatRequires();
const execArgv: string[] = [];
for (const r of requires) {
const module = this.formatImportModule(r);
// Remove the quotes from the path
// --require "module path" -> ['--require', 'module path']
// --import "module path" -> ['--import', 'module path']
const splitIndex = module.indexOf(' ');
if (splitIndex !== -1) {
execArgv.push(module.slice(0, splitIndex), module.slice(splitIndex + 2, -1));
Comment on lines +332 to +338
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

buildRequiresExecArgv() is constructing fork execArgv by generating a quoted CLI string (formatImportModule) and then slicing it back into ['--require', path]. This is brittle (depends on exact quoting/spacing) and can pass incorrectly escaped Windows paths (formatImportModule intentionally double-escapes backslashes for NODE_OPTIONS strings, which is not appropriate for execArgv arrays). Build the execArgv array directly (choose '--import' vs '--require' based on isESM, and pass the raw module specifier/path without adding/removing quotes).

Suggested change
const module = this.formatImportModule(r);
// Remove the quotes from the path
// --require "module path" -> ['--require', 'module path']
// --import "module path" -> ['--import', 'module path']
const splitIndex = module.indexOf(' ');
if (splitIndex !== -1) {
execArgv.push(module.slice(0, splitIndex), module.slice(splitIndex + 2, -1));
if (this.isESM) {
// For ESM, use --import with a file URL.
execArgv.push('--import', pathToFileURL(r).href);
} else {
// For CommonJS, use --require with the raw module path/specifier.
execArgv.push('--require', r);

Copilot uses AI. Check for mistakes.
}
}
return execArgv;
}

protected async forkNode(modulePath: string, forkArgs: string[], options: ForkNodeOptions = {}) {
const env = {
...this.env,
Expand Down
14 changes: 1 addition & 13 deletions tools/egg-bin/src/commands/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,7 @@ export default class Dev<T extends typeof Dev> extends BaseCommand<T> {
const serverBin = getSourceFilename(`../scripts/start-cluster.${ext}`);
const eggStartOptions = await this.formatEggStartOptions();
const args = [JSON.stringify(eggStartOptions)];
const requires = await this.formatRequires();
const execArgv: string[] = [];
for (const r of requires) {
const module = this.formatImportModule(r);

// Remove the quotes from the path
// --require "module path" -> ['--require', 'module path']
// --import "module path" -> ['--import', 'module path']
const splitIndex = module.indexOf(' ');
if (splitIndex !== -1) {
execArgv.push(module.slice(0, splitIndex), module.slice(splitIndex + 2, -1));
}
}
const execArgv = await this.buildRequiresExecArgv();
await this.forkNode(serverBin, args, { execArgv });
}

Expand Down
125 changes: 125 additions & 0 deletions tools/egg-bin/src/commands/manifest.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
import { debuglog } from 'node:util';

import { getFrameworkPath } from '@eggjs/utils';
import { Args, Flags } from '@oclif/core';

import { BaseCommand } from '../baseCommand.ts';
import { getSourceFilename } from '../utils.ts';

const debug = debuglog('egg/bin/commands/manifest');

export default class Manifest<T extends typeof Manifest> extends BaseCommand<T> {
static override description = 'Generate, validate, or clean the startup manifest for faster cold starts';

static override examples = [
'<%= config.bin %> <%= command.id %> generate',
'<%= config.bin %> <%= command.id %> generate --env=prod',
'<%= config.bin %> <%= command.id %> validate --env=prod',
'<%= config.bin %> <%= command.id %> clean',
];

static override args = {
action: Args.string({
required: true,
description: 'Action to perform',
options: ['generate', 'validate', 'clean'],
}),
};

static override flags = {
framework: Flags.string({
description: 'specify framework that can be absolute path or npm package',
}),
env: Flags.string({
description: 'server environment for manifest generation/validation',
default: 'prod',
Comment on lines +34 to +35
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The manifest command's default env flag is 'prod', while the E2E verification script (ecosystem-ci/scripts/verify-manifest.mjs) uses 'unittest' as its default. This discrepancy can lead to different behaviors between local testing and direct CLI usage, potentially causing confusion or requiring explicit --env flags in scenarios where consistency is expected. Consider aligning the default env value for consistency, or add a clear comment explaining the intentional difference.

Suggested change
description: 'server environment for manifest generation/validation',
default: 'prod',
env: Flags.string({
description: 'server environment for manifest generation/validation',
default: 'unittest',
}),

}),
scope: Flags.string({
description: 'server scope for manifest validation',
default: '',
}),
Comment on lines +37 to +40
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--scope is defined as a flag but generate doesn’t accept/propagate it, so users can validate a scoped manifest but can’t easily generate one for the same scope without setting EGG_SERVER_SCOPE manually. Consider supporting manifest generate --scope=... and propagating it into the generation subprocess.

Copilot uses AI. Check for mistakes.
};

public async run(): Promise<void> {
const { action } = this.args;
switch (action) {
case 'generate':
await this.runGenerate();
break;
case 'validate':
await this.runValidate();
break;
case 'clean':
await this.runClean();
break;
}
Comment on lines +43 to +55
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New manifest command adds non-trivial behavior (forking metadataOnly boot, validating manifests, cleaning) but there are existing command-level tests in tools/egg-bin/test/commands/*. Consider adding unit/integration tests for manifest generate/validate/clean to prevent regressions (e.g., correct framework resolution, scope/env handling, and exit codes).

Copilot uses AI. Check for mistakes.
}
Comment on lines +43 to +56
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new CLI command adds non-trivial behavior (generate/validate/clean, env handling, exit codes) but there are no egg-bin unit/integration tests added alongside the existing command test suite (tools/egg-bin/test/commands/*). Add a dedicated manifest command test file to cover at least: generate creates .egg/manifest.json, validate exits non-zero on missing/invalid manifest, and clean removes the manifest.

Copilot uses AI. Check for mistakes.

private async runGenerate(): Promise<void> {
const { flags } = this;
const framework = getFrameworkPath({
framework: flags.framework,
baseDir: flags.base,
});
debug('generate manifest: baseDir=%s, framework=%s, env=%s', flags.base, framework, flags.env);

const options = {
baseDir: flags.base,
framework,
env: flags.env,
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manifest generation options passed to manifest-generate.mjs don’t include scope, and the script only sets EGG_SERVER_ENV. If the app uses EGG_SERVER_SCOPE, the generated manifest may be tied to the default scope and won’t validate for other scopes. Consider including scope in the options and setting EGG_SERVER_SCOPE (or passing serverScope to framework.start).

Suggested change
debug('generate manifest: baseDir=%s, framework=%s, env=%s', flags.base, framework, flags.env);
const options = {
baseDir: flags.base,
framework,
env: flags.env,
debug(
'generate manifest: baseDir=%s, framework=%s, env=%s, scope=%s',
flags.base,
framework,
flags.env,
flags.scope,
);
const options = {
baseDir: flags.base,
framework,
env: flags.env,
scope: flags.scope,

Copilot uses AI. Check for mistakes.
};

const serverBin = getSourceFilename('../scripts/manifest-generate.mjs');
const args = [JSON.stringify(options)];
const execArgv = await this.buildRequiresExecArgv();
await this.forkNode(serverBin, args, { execArgv });
}
Comment on lines +43 to +83
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New egg-bin manifest functionality isn't covered by the existing tools/egg-bin/test/commands/* suite. Adding command-level tests (generate/validate/clean, plus a failure case when manifest is missing/invalid) would help prevent regressions, especially around env/scope/framework resolution.

Copilot uses AI. Check for mistakes.

private async runValidate(): Promise<void> {
const { flags } = this;
debug('validate manifest: baseDir=%s, env=%s, scope=%s', flags.base, flags.env, flags.scope);

// Bypass the local-env guard since the user explicitly asked to validate
const savedEggManifest = process.env.EGG_MANIFEST;
process.env.EGG_MANIFEST = 'true';

try {
const { ManifestStore } = await import('@eggjs/core');
const store = ManifestStore.load(flags.base, flags.env, flags.scope);
Comment on lines +93 to +95
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
echo "Check `@eggjs/core` imports in egg-bin:"
rg -n "import\\('@eggjs/core'\\)|importModule\\('@eggjs/core'" tools/egg-bin/src tools/egg-bin/scripts -C2

echo
echo "Check egg-bin dependency declaration for `@eggjs/core`:"
cat tools/egg-bin/package.json | sed -n '1,220p'

Repository: eggjs/egg

Length of output: 4471


Resolve @eggjs/core via framework path in validate and clean to ensure consistent module resolution

Lines 87 and 121 use bare import('@eggjs/core'), while runGenerate() properly resolves core from the selected framework path. In pnpm strict mode, this inconsistency can cause the wrong ManifestStore version to be used when validating or cleaning manifests from different framework versions, leading to compatibility issues.

Proposed fix
-import { getFrameworkPath } from '@eggjs/utils';
+import { getFrameworkPath, importModule } from '@eggjs/utils';
...
   private async runValidate(): Promise<void> {
     const { flags } = this;
     debug('validate manifest: baseDir=%s, env=%s, scope=%s', flags.base, flags.env, flags.scope);
+    const framework = getFrameworkPath({
+      framework: flags.framework,
+      baseDir: flags.base,
+    });
...
-      const { ManifestStore } = await import('@eggjs/core');
+      const { ManifestStore } = await importModule('@eggjs/core', {
+        paths: [framework],
+      });
...
   private async runClean(): Promise<void> {
     const { flags } = this;
     debug('clean manifest: baseDir=%s', flags.base);
+    const framework = getFrameworkPath({
+      framework: flags.framework,
+      baseDir: flags.base,
+    });
 
-    const { ManifestStore } = await import('@eggjs/core');
+    const { ManifestStore } = await importModule('@eggjs/core', {
+      paths: [framework],
+    });
     ManifestStore.clean(flags.base);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
try {
const { ManifestStore } = await import('@eggjs/core');
const store = ManifestStore.load(flags.base, flags.env, flags.scope);
import { BaseCommand, command, description, option } from 'egg-console';
import { join } from 'node:path';
import Debug from 'debug';
import { getFrameworkPath, importModule } from '@eggjs/utils';
import type { PkgInfo } from '@eggjs/core';
const debug = Debug('egg-bin');
export class Manifest extends BaseCommand {
static override get description() {
return 'Generate, validate, or clean framework manifest files for optimized startup and module discovery';
}
`@option`({
alias: 'm',
description: 'Manifest mode (generate, validate, or clean)',
default: 'generate',
})
manifest!: 'generate' | 'validate' | 'clean';
`@option`({
alias: 'b',
description: 'Base directory for manifest operations',
})
base!: string;
`@option`({
alias: 'e',
description: 'Environment (development, production, etc.)',
})
env!: string;
`@option`({
alias: 's',
description: 'Scope for manifest generation',
})
scope!: string;
`@option`({
alias: 'f',
description: 'Framework path',
})
framework!: string;
override async run(): Promise<void> {
const { manifest } = this;
switch (manifest) {
case 'generate':
await this.runGenerate();
break;
case 'validate':
await this.runValidate();
break;
case 'clean':
await this.runClean();
break;
default:
throw new Error(`Unknown manifest mode: ${manifest}`);
}
}
private async runGenerate(): Promise<void> {
const { flags } = this;
debug('generate manifest: baseDir=%s, env=%s, scope=%s', flags.base, flags.env, flags.scope);
const framework = getFrameworkPath({
framework: flags.framework,
baseDir: flags.base,
});
try {
const manifestGenerateScript = join(__dirname, '../scripts/manifest-generate.mjs');
const { fork } = await import('child_process');
return new Promise((resolve, reject) => {
const child = fork(manifestGenerateScript, [], {
stdio: 'inherit',
env: {
...process.env,
EGG_BIN_FRAMEWORK: framework,
EGG_BIN_BASE_DIR: flags.base,
EGG_BIN_ENV: flags.env,
EGG_BIN_SCOPE: flags.scope,
},
});
child.on('exit', (code) => {
if (code === 0) {
resolve();
} else {
reject(new Error(`manifest generate script exited with code ${code}`));
}
});
child.on('error', reject);
});
} catch (error) {
this.ctx.logger.error('generate manifest failed', error);
throw error;
}
}
private async runValidate(): Promise<void> {
const { flags } = this;
debug('validate manifest: baseDir=%s, env=%s, scope=%s', flags.base, flags.env, flags.scope);
const framework = getFrameworkPath({
framework: flags.framework,
baseDir: flags.base,
});
try {
const { ManifestStore } = await importModule('@eggjs/core', {
paths: [framework],
});
const store = ManifestStore.load(flags.base, flags.env, flags.scope);
if (!store.isValid()) {
throw new Error('Manifest validation failed');
}
this.ctx.console.log('✓ Manifest is valid');
} catch (error) {
this.ctx.logger.error('validate manifest failed', error);
throw error;
}
}
private async runClean(): Promise<void> {
const { flags } = this;
debug('clean manifest: baseDir=%s', flags.base);
const framework = getFrameworkPath({
framework: flags.framework,
baseDir: flags.base,
});
const { ManifestStore } = await importModule('@eggjs/core', {
paths: [framework],
});
ManifestStore.clean(flags.base);
this.ctx.console.log('✓ Manifest cleaned');
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tools/egg-bin/src/commands/manifest.ts` around lines 86 - 88, The validate
and clean flows import ManifestStore using a bare import('@eggjs/core') which
can resolve the wrong package in pnpm strict mode; change these to resolve and
import `@eggjs/core` from the selected framework path the same way runGenerate()
does (i.e., use the framework resolution helper used by runGenerate() to locate
the framework, then dynamic-import the framework's '@eggjs/core' and obtain
ManifestStore), and then call ManifestStore.load(flags.base, flags.env,
flags.scope) so validate, clean, and runGenerate all use the same resolved
ManifestStore implementation.


Comment on lines +94 to +96
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

manifest validate imports @eggjs/core via normal Node resolution, which can be a different version than the framework’s @eggjs/core (especially when --framework points at a different framework install). This can lead to validating a manifest with different validation logic than the runtime loader uses. Consider resolving ManifestStore relative to the resolved framework path (similar to manifest-generate.mjs), and/or actually using the --framework flag here so generation/validation are consistent.

Copilot uses AI. Check for mistakes.
if (!store) {
console.error('[manifest] Manifest is invalid or does not exist');
return this.exit(1);
}

const { data } = store;
const resolveCacheCount = Object.keys(data.resolveCache).length;
const fileDiscoveryCount = Object.keys(data.fileDiscovery).length;
const extensionCount = Object.keys(data.extensions).length;
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ManifestStore.load() currently doesn’t validate that resolveCache / fileDiscovery / extensions are present, so a manifest could pass load() but still have these fields missing. Calling Object.keys(data.resolveCache) would then throw. Consider defaulting to {} (or guarding) when computing these counts so validate fails gracefully for malformed manifests.

Suggested change
const resolveCacheCount = Object.keys(data.resolveCache).length;
const fileDiscoveryCount = Object.keys(data.fileDiscovery).length;
const extensionCount = Object.keys(data.extensions).length;
const resolveCacheCount = Object.keys(data.resolveCache ?? {}).length;
const fileDiscoveryCount = Object.keys(data.fileDiscovery ?? {}).length;
const extensionCount = Object.keys(data.extensions ?? {}).length;

Copilot uses AI. Check for mistakes.
console.log('[manifest] Manifest is valid');
console.log('[manifest] version: %d', data.version);
console.log('[manifest] generatedAt: %s', data.generatedAt);
console.log('[manifest] serverEnv: %s', data.invalidation.serverEnv);
console.log('[manifest] serverScope: %s', data.invalidation.serverScope);
console.log('[manifest] resolveCache entries: %d', resolveCacheCount);
console.log('[manifest] fileDiscovery entries: %d', fileDiscoveryCount);
console.log('[manifest] extension entries: %d', extensionCount);
} finally {
// Restore original env
if (savedEggManifest === undefined) {
delete process.env.EGG_MANIFEST;
} else {
process.env.EGG_MANIFEST = savedEggManifest;
}
}
}

private async runClean(): Promise<void> {
const { flags } = this;
debug('clean manifest: baseDir=%s', flags.base);

const { ManifestStore } = await import('@eggjs/core');
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

manifest clean also imports @eggjs/core from egg-bin’s dependency graph. If the goal is to operate on the manifest for a specific --framework, this can again pick a different ManifestStore implementation/version than the framework uses. Consider resolving ManifestStore from the framework path (or avoid importing core at all and delete the file directly).

Suggested change
const { ManifestStore } = await import('@eggjs/core');
const framework = getFrameworkPath({
framework: flags.framework,
baseDir: flags.base,
});
const frameworkModule: any = await import(framework);
const { ManifestStore } = frameworkModule;
if (!ManifestStore || typeof ManifestStore.clean !== 'function') {
throw new Error(`ManifestStore.clean is not available on framework module: ${framework}`);
}

Copilot uses AI. Check for mistakes.
ManifestStore.clean(flags.base);
Comment on lines +127 to +129
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clean also imports @eggjs/core without considering --framework. For consistency with generate (and to avoid pnpm strict / version mismatch issues), resolve and load ManifestStore from the same framework/core that owns the manifest, rather than egg-bin's own dependency graph.

Copilot uses AI. Check for mistakes.
console.log('[manifest] Manifest cleaned');
}
}
Loading
Loading