test: add e2e tests #5739
test: add e2e tests #5739
Conversation
|
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 ecosystem CI tooling: a composite GitHub Action to checkout projects, a matrix E2E workflow, repo metadata and TypeScript utilities to clone/sync repositories and patch package.json overrides, plus related .gitignore and package configuration updates. Changes
Sequence Diagram(s)sequenceDiagram
participant WF as GitHub Workflow
participant Action as Clone Composite Action
participant RepoJSON as ecosystem-ci/repo.json
participant Checkout as actions/checkout
participant Runner as Workflow Runner
WF->>Action: run with input ecosystem-ci-project
Action->>RepoJSON: read mapping for project
RepoJSON-->>Action: return repo URL, branch, commit
Action->>Action: set outputs (PROJECT_HASH, PROJECT_REPOSITORY)
Action->>Checkout: actions/checkout with repository + ref
Checkout->>Runner: place files under ecosystem-ci/<project>
Runner-->>WF: workflow continues using checked-out project dir
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
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 |
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 establishes the foundational infrastructure for end-to-end testing across an ecosystem of projects. It introduces a mechanism to programmatically clone specific versions of dependent repositories and then patch their Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Deploying egg with
|
| Latest commit: |
161b853
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0442d2df.egg-cci.pages.dev |
| Branch Preview URL: | https://e2e.egg-cci.pages.dev |
Deploying egg-v3 with
|
| Latest commit: |
1654355
|
| Status: | ⚡️ Build in progress... |
![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 introduces end-to-end tests for the ecosystem, which is a great addition for ensuring stability and compatibility. The implementation includes new GitHub Actions and helper scripts for cloning and patching test projects. I've identified a few areas for improvement, including a potential security vulnerability, a significant maintainability concern with a hardcoded list, and some opportunities for code refinement. My detailed feedback is provided in the comments below.
There was a problem hiding this comment.
Pull request overview
This PR adds an ecosystem-ci infrastructure for end-to-end testing of the Eggjs framework against downstream projects. It enables automated testing to ensure changes to the framework don't break dependent projects.
Key changes:
- Adds ecosystem-ci scripts for cloning and patching downstream repositories
- Implements a GitHub Actions workflow for automated E2E testing against cnpmcore
- Configures testing infrastructure with repository definitions and package management
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
ecosystem-ci/repo.json |
Defines downstream repositories for E2E testing (cnpmcore) |
ecosystem-ci/clone.ts |
Implements git repository cloning and checkout logic with validation |
ecosystem-ci/patch-project.ts |
Patches downstream projects to use local .tgz packages for testing |
ecosystem-ci/.gitignore |
Excludes cloned repositories from version control |
.gitignore |
Adds .tgz files to global ignore list |
.github/workflows/e2e-test.yml |
Defines E2E test workflow that runs on next branch |
.github/actions/clone/action.yml |
Reusable composite action for cloning ecosystem-ci repositories |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #5739 +/- ##
=======================================
Coverage 87.60% 87.61%
=======================================
Files 563 563
Lines 10930 10930
Branches 1238 1238
=======================================
+ Hits 9575 9576 +1
+ Misses 1271 1270 -1
Partials 84 84 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
…s.glob Replace hardcoded packages array with dynamic discovery based on pnpm-workspace.yaml patterns. This ensures newly added packages are automatically included in E2E testing without manual updates. - Parse pnpm-workspace.yaml to get workspace patterns - Use Node.js native glob API to find all package.json files - Skip private packages automatically - Add @types/js-yaml for TypeScript type support 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Co-authored-by: Copilot <[email protected]> Signed-off-by: MK (fengmk2) <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tsdown.config.ts (1)
1-33: Use a typed intermediate variable for the config export.Per coding guidelines, tsdown configuration files must use typed intermediate variables with explicit type annotations rather than exporting the config directly.
🔎 Proposed refactor to add typed intermediate variable
import { defineConfig } from 'tsdown'; +import type { Options } from 'tsdown'; -export default defineConfig({ +const config: Options = { // Workspace configuration - builds all library packages from root workspace: { include: ['packages/*', 'plugins/*', 'tools/*', 'tegg/core/*', 'tegg/plugin/*', 'tegg/standalone/*'], // FIXME: auto mode is not working, rolldown was hang and cpu 400% when using auto mode // include: 'auto', exclude: [ 'packages/tsconfig', // Config-only package, no src to build ], }, // Shared defaults unbundle: true, unused: { level: 'error', }, // dts: true, exports: { devExports: true, }, fixedExtension: false, publint: { level: 'suggestion', strict: true, }, // Default entry pattern - glob to include all source files entry: 'src/**/*.ts', skipNodeModulesBundle: true, external: [/^@eggjs\//, 'egg'], -}); +}; + +export default defineConfig(config);Based on coding guidelines, configuration files for tools (tsdown, vitest) must use typed intermediate variables with explicit type annotations.
♻️ Duplicate comments (9)
.github/workflows/e2e-test.yml (3)
15-17: The cancel-in-progress setting is always true.The workflow only triggers on the
nextbranch (lines 6, 11), so thecancel-in-progress: truesetting will always apply to all workflow runs. This is acceptable if you want to cancel in-progress runs when new commits are pushed to PRs targetingnext. However, the previous review comment suggested making this conditional based ongithub.ref_name != 'next', which would never be true for this workflow.If the current behavior is intentional, no change is needed. Otherwise, clarify the intended cancellation policy.
84-87: Step name could be more descriptive.The step "Override dependencies from tgz" accurately describes the operation being performed. However, a previous review suggested clarifying that this patches all eggjs packages. The current name is acceptable but could be more explicit.
89-91: Document the intentional use of npm in ecosystem projects.The workflow uses
npmcommands to run tests in the ecosystem-ci projects (line 91), while the main repository usespnpm. This is intentional to test compatibility with npm-based downstream consumers. As noted in a previous review, consider adding a comment to document this rationale.ecosystem-ci/patch-project.ts (3)
76-87: Refactor duplicated dependency patching logic.The logic for patching
dependencies(lines 76-81) anddevDependencies(lines 82-87) is identical. This duplication can be eliminated by iterating over both dependency types.🔎 Eliminate duplication
+ const depTypes = ['dependencies', 'devDependencies'] as const; + for (const depType of depTypes) { + const deps = packageJson[depType]; + if (!deps) continue; + for (const name in deps) { + const override = overrides[name]; + if (override) { + deps[name] = override; + } + } + } - for (const name in packageJson.dependencies) { - const override = overrides[name]; - if (override) { - packageJson.dependencies[name] = override; - } - } - for (const name in packageJson.devDependencies) { - const override = overrides[name]; - if (override) { - packageJson.devDependencies[name] = override; - } - }
17-20: Use consistent error message terminology.Line 18 says the project "is not defined in repo.json" while line 120 says it "is not supported". For consistency and clarity, both should use the same phrasing.
🔎 Make error messages consistent
default: - console.error(`Project ${project} is not supported`); + console.error(`Project ${project} is not defined in repo.json`); process.exit(1);Also applies to: 119-121
55-66: Add error handling for package.json reads.The
JSON.parsecall at line 60 can throw if a package.json file is malformed or missing the version field. This would crash the entire patching process rather than providing a useful error message identifying which package caused the issue.🔎 Add try-catch with descriptive error
async function buildOverrides(): Promise<Record<string, string>> { const packages = await discoverPackages(); const overrides: Record<string, string> = {}; for (const [name, path] of packages) { - const version = JSON.parse(fs.readFileSync(join(tgzPath, path, 'package.json'), 'utf8')).version; + try { + const pkgJson = JSON.parse(fs.readFileSync(join(tgzPath, path, 'package.json'), 'utf8')); + if (!pkgJson.version) { + console.warn(`Warning: Package ${name} at ${path} has no version field, skipping`); + continue; + } + const version = pkgJson.version; + } catch (error) { + console.error(`Error reading package.json for ${name} at ${path}:`, error.message); + throw error; + } const filename = `${name.replace('@', '').replace('/', '-')}-${version}.tgz`; overrides[name] = `file:${tgzPath}/${filename}`; } return overrides; }ecosystem-ci/clone.ts (2)
76-76: Handle null currentHash more gracefully.When
getCurrentHash()returnsnull, the expressioncurrentHash?.slice(0, 7)evaluates toundefined, resulting in a log message like"Current: undefined, expected: ...". Displaying 'unknown' would be more user-friendly.🔎 Display 'unknown' for null hash
- console.info(` → Current: ${currentHash?.slice(0, 7)}, expected: ${repo.hash.slice(0, 7)}`); + console.info(` → Current: ${currentHash?.slice(0, 7) ?? 'unknown'}, expected: ${repo.hash.slice(0, 7)}`);
9-11: The stdio configuration suppresses error output.The
stdio: ['pipe', 'pipe', 'pipe']configuration prevents stderr from being displayed, making it difficult to diagnose git command failures. While the function catches errors and returns null, users won't see what went wrong (e.g., network issues, invalid repository URLs).🔎 Allow stderr to be visible
function exec(cmd: string, execCwd: string = cwd): string { - return execSync(cmd, { cwd: execCwd, encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] }).trim(); + return execSync(cmd, { cwd: execCwd, encoding: 'utf-8', stdio: ['ignore', 'pipe', 'inherit'] }).trim(); }This configuration:
- Ignores stdin (not needed)
- Pipes stdout (for return value)
- Inherits stderr (shows errors to user)
.github/actions/clone/action.yml (1)
13-19: Security: Command injection vulnerability via input embedding.The
inputs.ecosystem-ci-projectvalue is directly embedded into the shell command at lines 18-19 without sanitization. If a malicious value is provided (e.g., viaworkflow_dispatch), it could execute arbitrary code.Additionally, the script uses CommonJS
require()in a project that follows ESM conventions elsewhere. Consider consolidating these calls into a single, safer operation.🔎 Safer implementation using environment variables and fs.readFileSync
- name: Output ecosystem ci project hash shell: bash id: ecosystem-ci-project-hash if: ${{ inputs.ecosystem-ci-project != '' }} + env: + PROJECT_NAME: ${{ inputs.ecosystem-ci-project }} run: | - node -e "const fs = require('fs'); const data = JSON.parse(fs.readFileSync('./ecosystem-ci/repo.json', 'utf8')); const project = data['${{ inputs.ecosystem-ci-project }}']; console.log('ECOSYSTEM_CI_PROJECT_HASH=' + project.hash);" >> $GITHUB_OUTPUT - node -e "const fs = require('fs'); const data = JSON.parse(fs.readFileSync('./ecosystem-ci/repo.json', 'utf8')); const project = data['${{ inputs.ecosystem-ci-project }}']; console.log('ECOSYSTEM_CI_PROJECT_REPOSITORY=' + project.repository.replace('https://github.com/', '').replace('.git', ''));" >> $GITHUB_OUTPUT + node -e "const fs = require('fs'); const data = JSON.parse(fs.readFileSync('./ecosystem-ci/repo.json', 'utf8')); const project = data[process.env.PROJECT_NAME]; if (!project) { console.error('Project not found'); process.exit(1); } console.log('ECOSYSTEM_CI_PROJECT_HASH=' + project.hash); console.log('ECOSYSTEM_CI_PROJECT_REPOSITORY=' + project.repository.replace('https://github.com/', '').replace('.git', ''));" >> $GITHUB_OUTPUTThis uses an environment variable to safely pass the input and combines both operations into a single
node -ecall.
🧹 Nitpick comments (3)
tsdown.config.ts (1)
7-8: Address the FIXME comment about auto mode.The comment indicates that auto mode causes rolldown to hang with 400% CPU usage. This issue should be investigated or tracked.
Do you want me to open a new issue to track this auto mode problem, or would you like me to help investigate potential workarounds?
package.json (1)
11-13: Consider removing the "files" property from the private package.The
"files"array typically controls which files are included when a package is published. Since this is a private monorepo root ("private": trueat line 4), it will never be published to npm. This property serves no functional purpose here and could be removed to avoid confusion.ecosystem-ci/clone.ts (1)
42-50: Consider validating branch and hash existence before operations.The
cloneRepoandcheckoutHashfunctions don't validate that the specified branch or commit hash exist in the remote repository. If the hash inrepo.jsonbecomes invalid (e.g., force-pushed away), the checkout will fail with a cryptic error.Consider adding validation or improving error messages to help diagnose such issues.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
.github/actions/clone/action.yml(1 hunks).github/workflows/e2e-test.yml(1 hunks).gitignore(1 hunks)ecosystem-ci/.gitignore(1 hunks)ecosystem-ci/clone.ts(1 hunks)ecosystem-ci/patch-project.ts(1 hunks)ecosystem-ci/repo.json(1 hunks)package.json(2 hunks)tegg/plugin/ajv/package.json(0 hunks)tsdown.config.ts(3 hunks)
💤 Files with no reviewable changes (1)
- tegg/plugin/ajv/package.json
🧰 Additional context used
📓 Path-based instructions (4)
package.json
📄 CodeRabbit inference engine (CLAUDE.md)
Use
workspace:*for internal package dependencies andcatalog:for external dependencies defined in pnpm-workspace.yaml
Files:
package.json
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
Use
.tssources over CommonJS and prefer ESM for exports
Files:
ecosystem-ci/patch-project.tsecosystem-ci/clone.tstsdown.config.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Name classes in PascalCase
Name functions and variables in camelCase
**/*.{ts,tsx}: Use explicit return types for all exported functions, methods, and getters to support TypeScript's--isolatedDeclarationsflag
Avoid computed property names with symbols in class declarations; use override methods instead for compatibility with--isolatedDeclarations
Add explicit type annotations for class properties, particularly for exported symbols which should useunique symboltype
Enable strict mode in all TypeScript packages and use explicit return types for public APIs
Prefer interfaces over type aliases for object shapes in TypeScript
Use readonly modifiers where appropriate in TypeScript classes and interfaces
Avoidanytype; useunknownwhen type is truly unknown
Files:
ecosystem-ci/patch-project.tsecosystem-ci/clone.tstsdown.config.ts
{tsdown,vitest}.config.ts
📄 CodeRabbit inference engine (CLAUDE.md)
{tsdown,vitest}.config.ts: Use typed intermediate variables for tsdown and vitest config exports with explicit UserConfig/UserWorkspaceConfig type annotations
Configuration files for tools (tsdown, vitest) must use typed intermediate variables with explicit type annotations for config objects
Files:
tsdown.config.ts
🧠 Learnings (47)
📓 Common learnings
Learnt from: CR
Repo: eggjs/egg PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-20T09:24:59.960Z
Learning: Ensure PRs pass lint, typecheck, and tests before submission
📚 Learning: 2025-12-21T03:04:53.982Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T03:04:53.982Z
Learning: All packages must include TypeScript type checking with `tsc --noEmit` in the `typecheck` script
Applied to files:
.github/actions/clone/action.ymlpackage.json
📚 Learning: 2025-12-20T09:24:59.960Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-20T09:24:59.960Z
Learning: Maintain Egg as a pnpm monorepo with core runtime code in `packages/egg`, supporting modules in `packages/core`, `packages/cluster`, and `packages/utils`, optional integrations under `plugins/`, examples in `examples/`, documentation site in `site/`, and unit tests beside packages under `test/` directories with fixtures in `test/fixtures/`
Applied to files:
package.jsonecosystem-ci/repo.json.github/workflows/e2e-test.ymlecosystem-ci/patch-project.tstsdown.config.ts
📚 Learning: 2025-12-21T03:04:53.982Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T03:04:53.982Z
Learning: Applies to plugins/*/package.json : All plugins must include egg in peerDependencies and define `build`, `clean`, and `prepublishOnly` scripts in package.json
Applied to files:
package.json
📚 Learning: 2025-12-21T03:04:53.982Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T03:04:53.982Z
Learning: All packages must define a `src/types.ts` file that extends the Egg module declarations using `declare module 'egg'` pattern
Applied to files:
package.jsontsdown.config.ts
📚 Learning: 2025-12-21T03:05:13.313Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-12-21T03:05:13.313Z
Learning: Applies to tegg/**/package.json : Egg.js version must be >= 4.1.0
Applied to files:
package.json.github/workflows/e2e-test.ymltsdown.config.ts
📚 Learning: 2025-11-27T14:37:59.711Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T14:37:59.711Z
Learning: Applies to pnpm-workspace.yaml : Define dependencies in pnpm-workspace.yaml catalog section and reference catalog entries with 'package-name': 'catalog:' or internal workspace dependencies with 'package-name': 'workspace:*'
Applied to files:
package.jsonecosystem-ci/patch-project.ts
📚 Learning: 2025-12-21T03:04:53.982Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T03:04:53.982Z
Learning: Applies to package.json : Use `workspace:*` for internal package dependencies and `catalog:` for external dependencies defined in pnpm-workspace.yaml
Applied to files:
package.json.github/workflows/e2e-test.ymlecosystem-ci/patch-project.tstsdown.config.ts
📚 Learning: 2025-12-21T03:05:13.313Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-12-21T03:05:13.313Z
Learning: Applies to tegg/**/pnpm-workspace.yaml : Use pnpm workspaces with `catalog:` protocol for shared external dependencies
Applied to files:
package.json.github/workflows/e2e-test.ymlecosystem-ci/patch-project.tstsdown.config.ts
📚 Learning: 2025-12-21T03:05:13.313Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-12-21T03:05:13.313Z
Learning: Applies to tegg/**/package.json : Use `workspace:*` protocol for internal monorepo dependencies (both tegg and egg packages)
Applied to files:
package.json.github/workflows/e2e-test.ymlecosystem-ci/patch-project.tstsdown.config.ts
📚 Learning: 2025-11-27T14:37:59.711Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T14:37:59.711Z
Learning: Applies to packages/**/*.ts : Use TypeScript throughout all packages in the Eggjs monorepo
Applied to files:
package.json.github/workflows/e2e-test.ymltsdown.config.ts
📚 Learning: 2025-12-21T03:05:13.313Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-12-21T03:05:13.313Z
Learning: Applies to tegg/**/pnpm-workspace.yaml : Tegg packages should be defined in root pnpm-workspace.yaml with patterns `tegg/core/*`, `tegg/plugin/*`, `tegg/standalone/*`
Applied to files:
package.jsonecosystem-ci/patch-project.tstsdown.config.ts.gitignore
📚 Learning: 2025-12-20T09:24:59.960Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-20T09:24:59.960Z
Learning: Keep local Node.js and pnpm versions aligned with the repository's `packageManager` field
Applied to files:
ecosystem-ci/repo.json.github/workflows/e2e-test.yml
📚 Learning: 2025-11-27T14:37:59.711Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T14:37:59.711Z
Learning: Run 'pnpm run test' to validate changes (set timeout to 180+ seconds for all tests, 120+ seconds for coverage) and never cancel the command
Applied to files:
.github/workflows/e2e-test.yml
📚 Learning: 2025-12-20T09:24:59.960Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-20T09:24:59.960Z
Learning: Add regression cases that exercise both CommonJS and TypeScript example apps when features affect HTTP or process orchestration
Applied to files:
.github/workflows/e2e-test.yml
📚 Learning: 2025-12-21T03:05:13.313Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-12-21T03:05:13.313Z
Learning: Applies to tegg/plugin/*/package.json : New plugins should define `eggPlugin` in `package.json` with dependencies
Applied to files:
.github/workflows/e2e-test.yml
📚 Learning: 2025-12-20T09:24:59.960Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-20T09:24:59.960Z
Learning: Run `pnpm install` to hydrate the workspace (Node.js ≥ 20.19.0 required)
Applied to files:
.github/workflows/e2e-test.ymlecosystem-ci/patch-project.ts
📚 Learning: 2025-11-27T14:37:59.711Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T14:37:59.711Z
Learning: Build all packages first using 'pnpm run build' before making changes, then validate with lint, tests, and documentation site checks
Applied to files:
.github/workflows/e2e-test.ymlecosystem-ci/patch-project.ts
📚 Learning: 2025-12-20T09:24:59.960Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-20T09:24:59.960Z
Learning: Use pnpm filters for package-specific workflows (e.g. `pnpm --filter=egg run test`)
Applied to files:
.github/workflows/e2e-test.ymlecosystem-ci/patch-project.ts
📚 Learning: 2025-12-20T09:24:59.960Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-20T09:24:59.960Z
Learning: Run `pnpm run test` for Vitest suite or `pnpm run test:cov` for coverage before submitting
Applied to files:
.github/workflows/e2e-test.yml
📚 Learning: 2025-12-20T09:24:59.960Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-20T09:24:59.960Z
Learning: Build all packages with `pnpm run build`
Applied to files:
.github/workflows/e2e-test.ymlecosystem-ci/patch-project.tstsdown.config.ts
📚 Learning: 2025-12-21T03:05:13.313Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-12-21T03:05:13.313Z
Learning: Applies to tegg/**/__tests__/**/*.test.{ts,tsx,js} : Use Vitest for testing all tegg packages
Applied to files:
.github/workflows/e2e-test.yml.gitignore
📚 Learning: 2025-12-21T03:04:53.982Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T03:04:53.982Z
Learning: Applies to packages/*/tsconfig.json : Create minimal tsconfig.json files in sub-packages containing only `{"extends": "../../tsconfig.json"}`
Applied to files:
ecosystem-ci/patch-project.ts
📚 Learning: 2025-12-21T03:05:13.313Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-12-21T03:05:13.313Z
Learning: Applies to tegg/**/pnpm-workspace.yaml : Centralize all shared dependency versions in the root `pnpm-workspace.yaml` rather than in individual tegg package files
Applied to files:
ecosystem-ci/patch-project.tstsdown.config.ts
📚 Learning: 2025-12-21T03:05:13.313Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-12-21T03:05:13.313Z
Learning: Applies to tegg/core/*/package.json : New core packages should include standard scripts in `package.json` with `typecheck` script using tsgo
Applied to files:
ecosystem-ci/patch-project.ts.gitignore
📚 Learning: 2025-12-21T03:04:53.982Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T03:04:53.982Z
Learning: Applies to **/tsconfig.json : Always extend the root tsconfig.json in sub-project TypeScript configurations using the minimal `extends` pattern
Applied to files:
ecosystem-ci/patch-project.ts
📚 Learning: 2025-12-21T03:04:53.982Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T03:04:53.982Z
Learning: Applies to packages/*/tsconfig.json : Sub-project TypeScript configurations must be minimal with only an `extends` field pointing to the root `tsconfig.json`; no additional options should be added
Applied to files:
ecosystem-ci/patch-project.tstsdown.config.ts
📚 Learning: 2025-12-21T03:04:53.982Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T03:04:53.982Z
Learning: Use `pnpm` workspace commands with `--filter` flag for package-specific operations instead of running commands in individual directories
Applied to files:
ecosystem-ci/patch-project.ts
📚 Learning: 2025-11-27T14:37:59.711Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T14:37:59.711Z
Learning: Run bootstrap and build commands in sequence: corepack enable pnpm → pnpm install → pnpm run build → pnpm run lint
Applied to files:
ecosystem-ci/patch-project.ts
📚 Learning: 2025-11-27T14:37:59.711Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T14:37:59.711Z
Learning: Applies to packages/*/tsdown.config.ts : Use tsdown for unbundled ESM builds that preserves file structure; configure via tsdown.config.ts in each package
Applied to files:
tsdown.config.ts
📚 Learning: 2025-12-21T03:04:53.982Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T03:04:53.982Z
Learning: Applies to {tsdown,vitest}.config.ts : Use typed intermediate variables for tsdown and vitest config exports with explicit UserConfig/UserWorkspaceConfig type annotations
Applied to files:
tsdown.config.ts
📚 Learning: 2025-12-21T03:04:53.982Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T03:04:53.982Z
Learning: Applies to {tsdown,vitest}.config.ts : Configuration files for tools (tsdown, vitest) must use typed intermediate variables with explicit type annotations for config objects
Applied to files:
tsdown.config.ts
📚 Learning: 2025-12-21T03:05:13.313Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-12-21T03:05:13.313Z
Learning: Applies to tegg/core/*/tsconfig.json : Each new core package should include `tsconfig.json` extending `eggjs/tsconfig`
Applied to files:
tsdown.config.ts.gitignore
📚 Learning: 2025-12-21T03:05:13.313Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-12-21T03:05:13.313Z
Learning: Applies to tegg/**/tsconfig.json : Use `emitDecoratorMetadata` in TypeScript configuration for type inference in dependency injection
Applied to files:
tsdown.config.ts
📚 Learning: 2025-12-20T09:24:59.960Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-20T09:24:59.960Z
Learning: Applies to **/*.{ts,tsx,js,mjs} : Use `.ts` sources over CommonJS and prefer ESM for exports
Applied to files:
tsdown.config.ts
📚 Learning: 2025-12-21T03:04:53.982Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T03:04:53.982Z
Learning: Most plugins do NOT need a `tsdown.config.ts` file - only create one if you need custom options like copying assets or custom entry points
Applied to files:
tsdown.config.ts
📚 Learning: 2025-12-21T03:04:53.982Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T03:04:53.982Z
Learning: Applies to **/*.{ts,tsx} : Use explicit return types for all exported functions, methods, and getters to support TypeScript's `--isolatedDeclarations` flag
Applied to files:
tsdown.config.ts
📚 Learning: 2025-12-21T03:05:13.313Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-12-21T03:05:13.313Z
Learning: Applies to tegg/**/vitest.config.ts : Configure test discovery through monorepo root `vitest.config.ts` which includes all tegg packages
Applied to files:
tsdown.config.ts.gitignore
📚 Learning: 2025-11-27T14:37:59.711Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T14:37:59.711Z
Learning: Applies to packages/**/*.ts : Use strict TypeScript mode in all packages
Applied to files:
tsdown.config.ts
📚 Learning: 2025-11-27T14:37:59.711Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T14:37:59.711Z
Learning: Applies to packages/*/tsconfig.json : All sub-project tsconfig.json files MUST extend from root tsconfig.json using 'extends': '../../tsconfig.json'
Applied to files:
tsdown.config.ts
📚 Learning: 2025-12-21T03:05:13.313Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-12-21T03:05:13.313Z
Learning: Applies to tegg/**/*.{js,ts,mjs,cjs} : Use ESM only (no CommonJS)
Applied to files:
tsdown.config.ts.gitignore
📚 Learning: 2025-12-21T03:04:53.982Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T03:04:53.982Z
Learning: Applies to plugins/*/package.json : Plugin packages must configure exports in package.json with both development (`./src/*.ts`) and published (`./dist/*.js`) entry points
Applied to files:
tsdown.config.ts
📚 Learning: 2025-12-21T03:05:13.313Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-12-21T03:05:13.313Z
Learning: Applies to tegg/**/*.{ts,tsx} : Do not use circular dependencies between prototypes or modules
Applied to files:
.gitignore
📚 Learning: 2025-12-21T03:05:13.313Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-12-21T03:05:13.313Z
Learning: Applies to tegg/**/*.{ts,tsx} : All imports should use `.js` extensions for ESM files
Applied to files:
.gitignore
📚 Learning: 2025-12-21T03:05:13.313Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-12-21T03:05:13.313Z
Learning: Applies to tegg/**/package.json : Node.js version must be >= 22.18.0
Applied to files:
.gitignore
📚 Learning: 2025-12-21T03:04:53.982Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T03:04:53.982Z
Learning: Applies to test/**/*.test.ts : Test files should follow the pattern `test/**/*.test.ts` and use vitest for new test files
Applied to files:
.gitignore
📚 Learning: 2025-12-20T09:24:59.960Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-20T09:24:59.960Z
Learning: Applies to **/test/**/*.test.ts : Configure Vitest to discover `**/test/**/*.test.ts` within each package and mirror this pattern when adding test suites
Applied to files:
.gitignore
🧬 Code graph analysis (1)
ecosystem-ci/patch-project.ts (1)
scripts/utils.js (1)
workspaceConfig(15-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Agent
- GitHub Check: Cloudflare Pages: egg-v3
🔇 Additional comments (10)
tsdown.config.ts (2)
32-32: External configuration looks correct.The external array properly excludes
@eggjs/scoped packages andeggfrom bundling, which is appropriate for a monorepo setup where these are workspace dependencies.
19-19: No action needed — declaration files are automatically generated through tsdown's auto-detection mechanism. If your package.json contains a types or typings field, declaration file generation will be enabled by default in tsdown. All packages declare the"types": "./dist/index.d.ts"field in their package.json, which triggers automatic .d.ts generation. Additionally, with isolatedDeclarations enabled, tsdown will use oxc-transform for .d.ts generation, which is extremely fast and highly recommended for optimal performance. The commented-outdts: trueis redundant but harmless.Likely an incorrect or invalid review comment.
ecosystem-ci/.gitignore (1)
1-2: LGTM!The ignore patterns correctly exclude the cloned ecosystem-ci project directories (cnpmcore and examples) that are managed by the clone script.
.gitignore (1)
105-106: LGTM!Adding
*.tgzcorrectly excludes packaged tarballs generated by thepnpm -r packcommand in the e2e workflow.package.json (1)
51-51: LGTM!Adding
@types/js-yamlas a dev dependency correctly provides TypeScript types for thejs-yamllibrary used inecosystem-ci/patch-project.ts. Thecatalog:protocol follows the coding guidelines for external dependencies.ecosystem-ci/patch-project.ts (3)
1-10: LGTM on imports and directory setup.The imports follow ESM conventions correctly, and the directory path setup using
import.meta.dirnameis appropriate.
94-107: LGTM on project-specific patching functions.The
patchCnpmcoreandpatchExamplesfunctions correctly apply overrides to their respective project package.json files. The examples function appropriately handles multiple sub-projects.
28-53: Add explicit return type and improve error handling.The
discoverPackagesfunction lacks an explicit return type annotation, which violates the coding guideline requiring explicit return types for all exported/public functions to support TypeScript's--isolatedDeclarationsflag.Additionally, the error handling at line 47 silently warns and continues. Consider whether skipping invalid package.json files is the desired behavior for all cases.
Based on coding guidelines, add explicit return type:
🔎 Add explicit return type annotation
-async function discoverPackages(): Promise<[string, string][]> { +async function discoverPackages(): Promise<Array<[string, string]>> {Or for better type safety:
type PackageInfo = [name: string, path: string]; async function discoverPackages(): Promise<PackageInfo[]> {⛔ Skipped due to learnings
Learnt from: CR Repo: eggjs/egg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-21T03:04:53.982Z Learning: Applies to **/*.{ts,tsx} : Use explicit return types for all exported functions, methods, and getters to support TypeScript's `--isolatedDeclarations` flagLearnt from: CR Repo: eggjs/egg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-21T03:04:53.982Z Learning: Applies to **/*.{ts,tsx} : Enable strict mode in all TypeScript packages and use explicit return types for public APIsLearnt from: CR Repo: eggjs/egg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-21T03:04:53.982Z Learning: Applies to **/*.{ts,tsx} : Add explicit type annotations for class properties, particularly for exported symbols which should use `unique symbol` typeLearnt from: CR Repo: eggjs/egg PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-11-27T14:37:59.711Z Learning: Applies to packages/**/*.ts : Use strict TypeScript mode in all packagesLearnt from: CR Repo: eggjs/egg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-21T03:04:53.982Z Learning: All packages must include TypeScript type checking with `tsc --noEmit` in the `typecheck` scriptLearnt from: CR Repo: eggjs/egg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-21T03:04:53.982Z Learning: Applies to **/*.{ts,tsx} : Avoid `any` type; use `unknown` when type is truly unknownLearnt from: CR Repo: eggjs/egg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-21T03:04:53.982Z Learning: Applies to package.json : Use `workspace:*` for internal package dependencies and `catalog:` for external dependencies defined in pnpm-workspace.yamlLearnt from: CR Repo: eggjs/egg PR: 0 File: tegg/CLAUDE.md:0-0 Timestamp: 2025-12-21T03:05:13.313Z Learning: Applies to tegg/**/pnpm-workspace.yaml : Tegg packages should be defined in root pnpm-workspace.yaml with patterns `tegg/core/*`, `tegg/plugin/*`, `tegg/standalone/*`Learnt from: CR Repo: eggjs/egg PR: 0 File: tegg/CLAUDE.md:0-0 Timestamp: 2025-12-21T03:05:13.313Z Learning: Applies to tegg/**/package.json : Use `workspace:*` protocol for internal monorepo dependencies (both tegg and egg packages)Learnt from: CR Repo: eggjs/egg PR: 0 File: tegg/CLAUDE.md:0-0 Timestamp: 2025-12-21T03:05:13.313Z Learning: Applies to tegg/**/pnpm-workspace.yaml : Use pnpm workspaces with `catalog:` protocol for shared external dependenciesLearnt from: CR Repo: eggjs/egg PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-11-27T14:37:59.711Z Learning: Applies to pnpm-workspace.yaml : Define dependencies in pnpm-workspace.yaml catalog section and reference catalog entries with 'package-name': 'catalog:' or internal workspace dependencies with 'package-name': 'workspace:*'ecosystem-ci/clone.ts (2)
53-85: LGTM on the main cloning and validation logic.The main loop correctly handles both existing and new repository directories, validates remote URLs with normalization, checks commit hashes, and performs cloning/checkout operations with appropriate logging. The error handling allows the script to continue processing other repositories even if one fails.
9-52: Add explicit return type annotations to all functions.The functions
exec,getRemoteUrl,normalizeGitUrl,isSameRepo,getCurrentHash,cloneRepo, andcheckoutHashlack explicit return type annotations. Per the coding guidelines, all functions should have explicit return types to support TypeScript's--isolatedDeclarationsflag.Based on coding guidelines:
🔎 Add explicit return types
-function exec(cmd: string, execCwd: string = cwd): string { +function exec(cmd: string, execCwd: string = cwd): string { // Already has return type ✓ } -function getRemoteUrl(dir: string): string | null { +function getRemoteUrl(dir: string): string | null { // Already has return type ✓ } -function normalizeGitUrl(url: string): string { +function normalizeGitUrl(url: string): string { // Already has return type ✓ } -function isSameRepo(url1: string, url2: string): boolean { +function isSameRepo(url1: string, url2: string): boolean { // Already has return type ✓ } -function getCurrentHash(dir: string): string | null { +function getCurrentHash(dir: string): string | null { // Already has return type ✓ } -function cloneRepo(repoUrl: string, branch: string, targetDir: string): void { +function cloneRepo(repoUrl: string, branch: string, targetDir: string): void { // Already has return type ✓ } -function checkoutHash(dir: string, hash: string): void { +function checkoutHash(dir: string, hash: string): void { // Already has return type ✓ }Actually, upon closer inspection, all functions already have explicit return types. No changes needed here.
⛔ Skipped due to learnings
Learnt from: CR Repo: eggjs/egg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-21T03:04:53.982Z Learning: Applies to **/*.{ts,tsx} : Use explicit return types for all exported functions, methods, and getters to support TypeScript's `--isolatedDeclarations` flagLearnt from: CR Repo: eggjs/egg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-21T03:04:53.982Z Learning: Applies to **/*.{ts,tsx} : Enable strict mode in all TypeScript packages and use explicit return types for public APIsLearnt from: CR Repo: eggjs/egg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-21T03:04:53.982Z Learning: Applies to **/*.{ts,tsx} : Add explicit type annotations for class properties, particularly for exported symbols which should use `unique symbol` type
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
| await patchExamples(overrides); | ||
| break; | ||
| default: | ||
| console.error(`Project ${project} is not supported`); |
There was a problem hiding this comment.
The error message when an unsupported project is encountered in the switch statement is inconsistent with the earlier validation. At this point, the project has already been validated to exist in repo.json (line 17-20), so this error message is misleading. Consider changing to something like console.error(\Project ${project} exists in repo.json but has no patch handler defined`)` to accurately reflect the issue.
| console.error(`Project ${project} is not supported`); | |
| console.error(`Project ${project} exists in repo.json but has no patch handler defined`); |
| function cloneRepo(repoUrl: string, branch: string, targetDir: string): void { | ||
| console.info(`Cloning ${repoUrl} (branch: ${branch})...`); | ||
| exec(`git clone --branch ${branch} ${repoUrl} ${targetDir}`); |
There was a problem hiding this comment.
The git command construction in cloneRepo is vulnerable to command injection. The repoUrl, branch, and targetDir parameters are directly interpolated into the shell command without sanitization. While currently these values come from a static JSON file, if the source of these values changes in the future, malicious input could execute arbitrary commands. Use array syntax for execSync or properly escape/validate these parameters to prevent potential command injection.
| } catch { | ||
| // Skip if package.json is invalid or cannot be read | ||
| console.warn(`Warning: Could not read ${pkgJsonPath}`); | ||
| } |
There was a problem hiding this comment.
The error handling in this catch block silently ignores all errors by logging a warning and continuing. If package.json parsing fails for important packages, the script will silently skip them without clearly indicating the impact. Consider either failing fast when critical packages cannot be read, or collecting and reporting all warnings at the end so users are aware of skipped packages.
Co-authored-by: Copilot <[email protected]> Signed-off-by: MK (fengmk2) <[email protected]>
| // FIXME: auto mode is not working, rolldown was hang and cpu 400% when using auto mode | ||
| // include: 'auto', |
There was a problem hiding this comment.
The comment states "FIXME: auto mode is not working" which indicates an unresolved issue. This commented-out code should either be removed if not needed, or the underlying issue should be fixed before merging.
| // FIXME: auto mode is not working, rolldown was hang and cpu 400% when using auto mode | |
| // include: 'auto', |
| "files": [ | ||
| "README.md" | ||
| ], |
There was a problem hiding this comment.
Adding a "files" field with only "README.md" to a private monorepo package is unnecessary since private packages are never published to npm. This field has no effect and should be removed to avoid confusion.
| "files": [ | |
| "README.md" | |
| ], |
| } | ||
|
|
||
| const packageJsonString = JSON.stringify(packageJson, null, 2) + '\n'; | ||
| console.log(packageJsonString); |
There was a problem hiding this comment.
The console.log output on line 90 will be mixed with the actual execution output, making it difficult to distinguish between debug information and actual results. Consider using console.error for debug output or adding a flag to control verbosity.
| console.log(packageJsonString); | |
| console.error(packageJsonString); |
| } | ||
| } | ||
|
|
||
| main(); |
There was a problem hiding this comment.
Missing error handling for the main function. If any of the async operations fail, the process will exit with a success code (0) by default. Add a catch block to ensure the process exits with a non-zero code on failure.
| main(); | |
| main().catch((err) => { | |
| console.error(err); | |
| process.exitCode = 1; | |
| }); |
| function cloneRepo(repoUrl: string, branch: string, targetDir: string): void { | ||
| console.info(`Cloning ${repoUrl} (branch: ${branch})...`); | ||
| exec(`git clone --branch ${branch} ${repoUrl} ${targetDir}`); | ||
| } | ||
|
|
||
| function checkoutHash(dir: string, hash: string): void { | ||
| console.info(`Checking out ${hash}...`); | ||
| exec(`git fetch origin`, dir); | ||
| exec(`git checkout ${hash}`, dir); |
There was a problem hiding this comment.
Potential command injection vulnerability. The git commands use unsanitized input from repo.json (branch, repository URL, and hash). While these are configuration values, if repo.json is ever modified maliciously, this could lead to command injection. Consider validating or sanitizing these inputs before using them in shell commands.
| function cloneRepo(repoUrl: string, branch: string, targetDir: string): void { | |
| console.info(`Cloning ${repoUrl} (branch: ${branch})...`); | |
| exec(`git clone --branch ${branch} ${repoUrl} ${targetDir}`); | |
| } | |
| function checkoutHash(dir: string, hash: string): void { | |
| console.info(`Checking out ${hash}...`); | |
| exec(`git fetch origin`, dir); | |
| exec(`git checkout ${hash}`, dir); | |
| function validateGitUrl(name: string, url: string): string { | |
| // Allow typical Git URL characters, disallow whitespace and shell metacharacters. | |
| const gitUrlPattern = /^[A-Za-z0-9@._:/%+-]+$/; | |
| if (!gitUrlPattern.test(url)) { | |
| throw new Error(`Invalid ${name}: contains unsafe characters`); | |
| } | |
| return url; | |
| } | |
| function validateGitRef(name: string, ref: string): string { | |
| // Allow typical Git ref characters (no spaces or shell metacharacters). | |
| const gitRefPattern = /^[A-Za-z0-9._\/\-]+$/; | |
| if (!gitRefPattern.test(ref)) { | |
| throw new Error(`Invalid ${name}: contains unsafe characters`); | |
| } | |
| return ref; | |
| } | |
| function validateGitHash(name: string, hash: string): string { | |
| // Git hashes are hex strings, usually 40 chars, but allow shorter prefixes. | |
| const gitHashPattern = /^[0-9a-fA-F]{7,40}$/; | |
| if (!gitHashPattern.test(hash)) { | |
| throw new Error(`Invalid ${name}: must be a hex commit hash`); | |
| } | |
| return hash; | |
| } | |
| function cloneRepo(repoUrl: string, branch: string, targetDir: string): void { | |
| const safeRepoUrl = validateGitUrl('repository URL', repoUrl); | |
| const safeBranch = validateGitRef('branch', branch); | |
| console.info(`Cloning ${safeRepoUrl} (branch: ${safeBranch})...`); | |
| exec(`git clone --branch ${safeBranch} ${safeRepoUrl} ${targetDir}`); | |
| } | |
| function checkoutHash(dir: string, hash: string): void { | |
| const safeHash = validateGitHash('hash', hash); | |
| console.info(`Checking out ${safeHash}...`); | |
| exec(`git fetch origin`, dir); | |
| exec(`git checkout ${safeHash}`, dir); |
| for (const [repoName, repo] of Object.entries(repos)) { | ||
| const targetDir = join(cwd, repoName); | ||
|
|
||
| if (existsSync(targetDir)) { | ||
| console.info(`\nDirectory ${repoName} exists, validating...`); | ||
|
|
||
| const remoteUrl = getRemoteUrl(targetDir); | ||
| if (!remoteUrl) { | ||
| console.error(` ✗ ${repoName} is not a git repository`); | ||
| continue; | ||
| } | ||
|
|
||
| if (!isSameRepo(remoteUrl, repo.repository)) { | ||
| console.error(` ✗ Remote mismatch: expected ${repo.repository}, got ${remoteUrl}`); | ||
| continue; | ||
| } | ||
|
|
||
| console.info(` ✓ Remote matches`); | ||
|
|
||
| const currentHash = getCurrentHash(targetDir); | ||
| if (currentHash === repo.hash) { | ||
| console.info(` ✓ Already at correct commit ${repo.hash.slice(0, 7)}`); | ||
| } else { | ||
| console.info(` → Current: ${currentHash?.slice(0, 7)}, expected: ${repo.hash.slice(0, 7)}`); | ||
| checkoutHash(targetDir, repo.hash); | ||
| console.info(` ✓ Checked out ${repo.hash.slice(0, 7)}`); | ||
| } | ||
| } else { | ||
| cloneRepo(repo.repository, repo.branch, targetDir); | ||
| checkoutHash(targetDir, repo.hash); | ||
| console.info(`✓ Cloned and checked out ${repo.hash.slice(0, 7)}`); | ||
| } | ||
| } | ||
|
|
||
| console.info('\nDone!'); |
There was a problem hiding this comment.
Missing error handling for git command failures. When cloneRepo or checkoutHash fail (lines 81-82), the script continues execution and prints "Done!" even though the operation may have failed. This could lead to false positives in CI/CD. Consider wrapping the main loop in a try-catch block or tracking failures to exit with an appropriate error code.
| for (const [repoName, repo] of Object.entries(repos)) { | |
| const targetDir = join(cwd, repoName); | |
| if (existsSync(targetDir)) { | |
| console.info(`\nDirectory ${repoName} exists, validating...`); | |
| const remoteUrl = getRemoteUrl(targetDir); | |
| if (!remoteUrl) { | |
| console.error(` ✗ ${repoName} is not a git repository`); | |
| continue; | |
| } | |
| if (!isSameRepo(remoteUrl, repo.repository)) { | |
| console.error(` ✗ Remote mismatch: expected ${repo.repository}, got ${remoteUrl}`); | |
| continue; | |
| } | |
| console.info(` ✓ Remote matches`); | |
| const currentHash = getCurrentHash(targetDir); | |
| if (currentHash === repo.hash) { | |
| console.info(` ✓ Already at correct commit ${repo.hash.slice(0, 7)}`); | |
| } else { | |
| console.info(` → Current: ${currentHash?.slice(0, 7)}, expected: ${repo.hash.slice(0, 7)}`); | |
| checkoutHash(targetDir, repo.hash); | |
| console.info(` ✓ Checked out ${repo.hash.slice(0, 7)}`); | |
| } | |
| } else { | |
| cloneRepo(repo.repository, repo.branch, targetDir); | |
| checkoutHash(targetDir, repo.hash); | |
| console.info(`✓ Cloned and checked out ${repo.hash.slice(0, 7)}`); | |
| } | |
| } | |
| console.info('\nDone!'); | |
| let hasError = false; | |
| for (const [repoName, repo] of Object.entries(repos)) { | |
| const targetDir = join(cwd, repoName); | |
| try { | |
| if (existsSync(targetDir)) { | |
| console.info(`\nDirectory ${repoName} exists, validating...`); | |
| const remoteUrl = getRemoteUrl(targetDir); | |
| if (!remoteUrl) { | |
| console.error(` ✗ ${repoName} is not a git repository`); | |
| continue; | |
| } | |
| if (!isSameRepo(remoteUrl, repo.repository)) { | |
| console.error(` ✗ Remote mismatch: expected ${repo.repository}, got ${remoteUrl}`); | |
| continue; | |
| } | |
| console.info(` ✓ Remote matches`); | |
| const currentHash = getCurrentHash(targetDir); | |
| if (currentHash === repo.hash) { | |
| console.info(` ✓ Already at correct commit ${repo.hash.slice(0, 7)}`); | |
| } else { | |
| console.info(` → Current: ${currentHash?.slice(0, 7)}, expected: ${repo.hash.slice(0, 7)}`); | |
| checkoutHash(targetDir, repo.hash); | |
| console.info(` ✓ Checked out ${repo.hash.slice(0, 7)}`); | |
| } | |
| } else { | |
| cloneRepo(repo.repository, repo.branch, targetDir); | |
| checkoutHash(targetDir, repo.hash); | |
| console.info(`✓ Cloned and checked out ${repo.hash.slice(0, 7)}`); | |
| } | |
| } catch (error) { | |
| hasError = true; | |
| console.error(`\n✗ Failed to process ${repoName}:`, error); | |
| } | |
| } | |
| if (hasError) { | |
| console.error('\nDone with errors.'); | |
| process.exitCode = 1; | |
| } else { | |
| console.info('\nDone!'); | |
| } |
| level: 'error', | ||
| }, | ||
| dts: true, | ||
| // dts: true, |
There was a problem hiding this comment.
The dts generation is commented out which will prevent TypeScript declaration files from being generated. This will break TypeScript support for consumers of these packages. If this is intentional for testing purposes, it should be reverted before merging or documented why it's disabled.
| // dts: true, | |
| dts: true, |
Summary by CodeRabbit
New Features
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.