Skip to content

Conversation

@fengmk2
Copy link
Member

@fengmk2 fengmk2 commented Oct 30, 2025

Summary by CodeRabbit

  • Chores
    • Updated several development tool versions and adjusted workspace exclude lists and version notations.
  • Performance
    • Expanded parallel execution for build/typecheck/pretest flows and added pre-clean sequencing to improve run-time efficiency.
  • Tests
    • Test emissions standardized for consistency and flaky test suites now skip automatically on Windows to reduce instability.

@fengmk2 fengmk2 requested a review from Copilot October 30, 2025 12:58
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @fengmk2, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request primarily focuses on maintaining the project's dependency health by updating various packages to their latest stable versions. These updates encompass type definitions, code minifiers, formatters, linters, and bundler-related tools, ensuring compatibility and leveraging recent improvements across the development stack. Additionally, the pnpm workspace configuration has been adjusted to align with these dependency changes.

Highlights

  • Core Dependency Updates: Several key dependencies including '@types/node', 'oxc-minify', 'oxfmt', 'oxlint', 'oxlint-tsgolint', 'rolldown-vite', and 'rolldown' have been updated to their latest versions, ensuring the project benefits from bug fixes, performance improvements, and new features.
  • Keygrip Deprecation Status: The 'deprecated' flag for the 'keygrip' package has been removed, indicating it might no longer be considered deprecated or the warning was removed.
  • Pnpm Workspace Configuration: The 'pnpm-workspace.yaml' file has been updated to reflect the new '@types/node' and 'oxlint' versions in the catalog, and 'oxlint-tsgolint', 'oxlint', and 'oxc-minify' have been added to the 'minimumReleaseAgeExclude' list.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 30, 2025

Note

Other AI code review bot(s) detected

CodeRabbit 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.

Walkthrough

Update workspace catalog and exclusion lists in pnpm-workspace.yaml, enable/expand parallel root pnpm script execution in package.json, change IPC test emits to use undefined instead of null, and conditionally skip several flaky schedule tests on Windows.

Changes

Cohort / File(s) Summary
Workspace config
pnpm-workspace.yaml
Catalog version/spec updates (@types/node ^24.9.1 → ^24.9.2; oxc-minify, oxlint, oxlint-tsgolint version spec changes) and added entries to minimumReleaseAgeExclude (@oxlint/*, oxlint-tsgolint, oxlint, oxc-minify).
Root scripts
package.json
Root scripts changed to use parallel recursive execution and pre-clean sequencing: clean uses pnpm -r --parallel run clean; build now runs pnpm clean && pnpm -r run build; added build:parallel (pnpm clean && pnpm -r --parallel run build); added typecheck:parallel; preci uses pnpm -r --parallel run pretest.
Tests — IPC
packages/egg/test/lib/core/messenger/ipc.test.ts
Replaced null with undefined in process.emit('message', ...) calls and related test emits to represent absent message data as undefined.
Tests — schedule (Windows skip)
plugins/schedule/test/cronError.test.ts, plugins/schedule/test/customType.test.ts, plugins/schedule/test/executeError.test.ts
Top-level describe blocks converted to describe.skipIf(process.platform === 'win32')(...) with comments noting Windows flakiness/timeouts; tests otherwise unchanged.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Dev as Developer (runs script)
  participant Root as pnpm (root)
  participant PkgA as Package A
  participant PkgB as Package B
  Note over Root: Updated root script behavior — pre-clean + optional --parallel
  Dev->>Root: pnpm clean && pnpm -r --parallel run build
  par Parallel package builds
    Root->>PkgA: run build
    Root->>PkgB: run build
  end
  PkgA-->>Root: build complete
  PkgB-->>Root: build complete
  Root-->>Dev: exit
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Review focus:
    • pnpm-workspace.yaml — verify version spec changes and minimumReleaseAgeExclude formatting/intent.
    • package.json scripts — ensure tasks safe to run with --parallel and pre-clean sequencing.
    • Modified tests — confirm semantics of undefined vs null in IPC test and that Windows skips are appropriate.

Possibly related PRs

Poem

🐰 I hopped through YAML, neat and bright,
Bumped versions, set scripts to sprint in flight,
Tests now skip where Windows makes them sigh,
Messages whisper "undefined" — not shy,
A small rabbit patch: build, skip, and delight. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The pull request title "chore: update deps" is partially related to the changeset but misleading about its full scope. While dependency version updates are present in pnpm-workspace.yaml, the changeset contains substantial additional changes not captured by the title: significant script modifications in package.json that introduce parallelism features (clean, build, build:parallel, typecheck, typecheck:parallel, preci), and test fixes across multiple test files (ipc.test.ts, cronError.test.ts, customType.test.ts, executeError.test.ts) that address test flakiness and null/undefined handling. A developer scanning commit history with this title would not understand the full scope of work being performed, particularly the build system changes and test reliability improvements. Consider revising the title to comprehensively describe all major changes, such as "chore: update deps, enable parallel build scripts, and fix test flakiness on Windows" or similar wording that captures the dependency updates, build system changes, and test reliability improvements. This would provide reviewers and future developers with a clearer understanding of what this PR accomplishes at a glance.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch update-deps

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates several dependencies. While most changes seem correct, there are a few inconsistencies in pnpm-lock.yaml that should be addressed. Specifically, the catalogs section has version mismatches for @types/node and oxlint where the resolved version does not match the updated specifier. Additionally, the deprecated flag for the keygrip package has been removed, which is not ideal as it hides important information from developers. These issues suggest the lock file might be in an inconsistent state. It is recommended to regenerate the pnpm-lock.yaml file by running pnpm install to ensure its integrity.

pnpm-lock.yaml Outdated
'@types/node':
specifier: ^24.9.1
specifier: ^24.9.2
version: 24.9.1
Copy link
Contributor

Choose a reason for hiding this comment

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

high

There's an inconsistency in the lock file for @types/node. The specifier has been updated to ^24.9.2, but the resolved version is still 24.9.1. This should be 24.9.2 to match the specifier and other parts of the lock file where this dependency is used. This might indicate an incomplete dependency update.

      version: 24.9.2

oxlint:
specifier: ^1.24.0
specifier: ^1.25.0
version: 1.24.0
Copy link
Contributor

Choose a reason for hiding this comment

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

high

Similar to the issue with @types/node, there's an inconsistency here for oxlint. The specifier is ^1.25.0 but the resolved version is 1.24.0. This should be updated to 1.25.0 to maintain consistency in the lock file.

      version: 1.25.0

[email protected]:
resolution: {integrity: sha512-iYSchDJ+liQ8iwbSI2QqsQOvqv58eJCEanyJPJi+Khyu8smkcKSFUCbPwzFcL7YVtZ6eONjqRX/38caJ7QjRAQ==}
engines: {node: '>= 0.6'}
deprecated: Package no longer supported. Contact Support at https://www.npmjs.com/support for more info.
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The deprecated field for [email protected] was removed. This package is indeed deprecated, and this field provides a useful warning to developers. It's better to keep it in the lockfile. Was this removal intentional?

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Oct 30, 2025

Deploying egg with  Cloudflare Pages  Cloudflare Pages

Latest commit: 4021a81
Status: ✅  Deploy successful!
Preview URL: https://5426b117.egg-cci.pages.dev
Branch Preview URL: https://update-deps.egg-cci.pages.dev

View logs

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Oct 30, 2025

Deploying egg-v3 with  Cloudflare Pages  Cloudflare Pages

Latest commit: 4021a81
Status: ✅  Deploy successful!
Preview URL: https://112a7c5d.egg-v3.pages.dev
Branch Preview URL: https://update-deps.egg-v3.pages.dev

View logs

@codecov
Copy link

codecov bot commented Oct 30, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.43%. Comparing base (57751a0) to head (4021a81).
⚠️ Report is 1 commits behind head on next.

Additional details and impacted files
@@            Coverage Diff             @@
##             next    #5671      +/-   ##
==========================================
+ Coverage   85.42%   87.43%   +2.00%     
==========================================
  Files         561      561              
  Lines       10931    10931              
  Branches     1238     1238              
==========================================
+ Hits         9338     9557     +219     
+ Misses       1494     1290     -204     
+ Partials       99       84      -15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates several development dependencies in the monorepo's catalog configuration. The changes include version bumps for TypeScript types and build/linting tools.

  • Updates @types/node from ^24.9.1 to ^24.9.2
  • Updates oxlint from ^1.24.0 to ^1.25.0
  • Updates oxfmt from ^0.8.0 to ^0.9.0
  • Updates oxc-minify from ^0.95.0 to ^0.96.0
  • Updates oxlint-tsgolint from ^0.3.0 to ^0.4.0
  • Adds oxlint-tsgolint, oxlint, and oxc-minify to the minimumReleaseAgeExclude list

Reviewed Changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 2 comments.

File Description
pnpm-workspace.yaml Updates catalog versions for @types/node, oxlint, oxfmt, oxc-minify, oxlint-tsgolint; adds these tools to minimumReleaseAgeExclude
pnpm-lock.yaml Updates lockfile with resolved versions and dependency graph changes corresponding to catalog updates
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

pnpm-lock.yaml Outdated
'@types/node':
specifier: ^24.9.1
specifier: ^24.9.2
version: 24.9.1
Copy link

Copilot AI Oct 30, 2025

Choose a reason for hiding this comment

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

The catalog specifies @types/node version ^24.9.2, but the lockfile shows version 24.9.1 is actually installed. This indicates the lockfile is out of sync with the catalog specification. Run pnpm install to update the lockfile to install version 24.9.2.

Suggested change
version: 24.9.1
version: 24.9.2

Copilot uses AI. Check for mistakes.
oxlint:
specifier: ^1.24.0
specifier: ^1.25.0
version: 1.24.0
Copy link

Copilot AI Oct 30, 2025

Choose a reason for hiding this comment

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

The catalog specifies oxlint version ^1.25.0, but the lockfile shows version 1.24.0 is actually installed. This indicates the lockfile is out of sync with the catalog specification. Run pnpm install to update the lockfile to install version 1.25.0.

Suggested change
version: 1.24.0
version: 1.25.0

Copilot uses AI. Check for mistakes.
@socket-security
Copy link

socket-security bot commented Oct 31, 2025

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.

View full report

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between efaa265 and 4021a81.

📒 Files selected for processing (4)
  • package.json (1 hunks)
  • plugins/schedule/test/cronError.test.ts (1 hunks)
  • plugins/schedule/test/customType.test.ts (1 hunks)
  • plugins/schedule/test/executeError.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

**/*.ts: Prefer TypeScript and ESM: write sources and exports in .ts (ESM-first) rather than CommonJS
Use two-space indentation, trailing commas, and semicolons (Prettier/oxlint defaults)
Name files in lowercase with hyphens (e.g., loader-context.ts)
Name classes in PascalCase
Name functions and variables in camelCase
Re-export types thoughtfully to keep the public API stable

Files:

  • plugins/schedule/test/customType.test.ts
  • plugins/schedule/test/executeError.test.ts
  • plugins/schedule/test/cronError.test.ts
**/test/**/*.test.ts

📄 CodeRabbit inference engine (AGENTS.md)

**/test/**/*.test.ts: Place test suites following Vitest discovery: /test//*.test.ts
Mirror the repository test pattern when adding new suites

Files:

  • plugins/schedule/test/customType.test.ts
  • plugins/schedule/test/executeError.test.ts
  • plugins/schedule/test/cronError.test.ts
{packages/**,plugins/**,tools/!(egg-bin)/**}/**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

{packages/**,plugins/**,tools/!(egg-bin)/**}/**/*.ts: For isolatedDeclarations support, all exported functions/methods/getters must have explicit return type annotations
Avoid computed property names with symbols in class declarations (no get SYM)
Add explicit type annotations for class properties when needed (no inferred exported property types)
Exported symbols must use unique symbol type (e.g., export const X: unique symbol = Symbol('x'))

Files:

  • plugins/schedule/test/customType.test.ts
  • plugins/schedule/test/executeError.test.ts
  • plugins/schedule/test/cronError.test.ts
{packages/!(cookies)/**/test/**/*.test.ts,plugins/**/test/**/*.test.ts}

📄 CodeRabbit inference engine (CLAUDE.md)

Vitest test files must follow the naming pattern test/**/*.test.ts

Files:

  • plugins/schedule/test/customType.test.ts
  • plugins/schedule/test/executeError.test.ts
  • plugins/schedule/test/cronError.test.ts
{packages/**/test/**,plugins/**/test/**}

📄 CodeRabbit inference engine (CLAUDE.md)

Use Node.js assert for assertions in Vitest tests

Files:

  • plugins/schedule/test/customType.test.ts
  • plugins/schedule/test/executeError.test.ts
  • plugins/schedule/test/cronError.test.ts
{package.json,packages/**/package.json,plugins/**/package.json,tools/**/package.json}

📄 CodeRabbit inference engine (CLAUDE.md)

All packages must require Node.js >= 22.18.0 (set engines.node to ">=22.18.0")

Files:

  • package.json
🧠 Learnings (22)
📓 Common learnings
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-10-23T17:37:37.567Z
Learning: Use pnpm workspaces with catalog: for shared externals and workspace:* for internal deps; centralize versions in root pnpm-workspace.yaml; set catalogMode: prefer in root .npmrc
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-18T11:59:58.226Z
Learning: Applies to {packages/**/package.json,plugins/**/package.json,tools/**/package.json} : Use catalog: versions for external dependencies defined in pnpm-workspace.yaml
Learnt from: CR
Repo: eggjs/egg PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to pnpm-workspace.yaml : Define dependency versions via the catalog section in pnpm-workspace.yaml and reference them with "catalog:"
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-10-23T17:37:37.567Z
Learning: Applies to tegg/{core,plugin,standalone}/*/package.json : Use workspace:* for internal monorepo dependencies and catalog: for external dependencies
Learnt from: CR
Repo: eggjs/egg PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T07:56:17.417Z
Learning: Run pnpm run test locally before submitting changes
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-18T11:59:58.226Z
Learning: Applies to {packages/**/package.json,plugins/**/package.json,tools/**/package.json} : Include script "lint:fix" that runs oxlint with --fix (e.g., "pnpm run lint -- --fix")
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-18T11:59:58.226Z
Learning: Applies to {packages/**/package.json,plugins/**/package.json,tools/**/package.json} : Use workspace:* for internal dependencies between monorepo packages
Learnt from: CR
Repo: eggjs/egg PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to packages/*/package.json : Use "workspace:*" for internal workspace dependencies in package.json
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-18T11:59:58.226Z
Learning: Applies to plugins/*/package.json : Plugins must include scripts: build (tsdown && rimraf dist *.tsbuildinfo && tsc -p tsconfig.build.json), typecheck (tsc --noEmit), lint (oxlint --type-aware), test (vitest run), prepublishOnly (pnpm run build)
Learnt from: CR
Repo: eggjs/egg PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T07:56:17.417Z
Learning: Use pnpm with Node.js ≥ 20.19.0; keep local tool versions aligned with the repository packageManager
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to packages/**/test/**/*.test.ts : Use import { describe, it } from 'vitest' in tests

Applied to files:

  • plugins/schedule/test/customType.test.ts
  • plugins/schedule/test/executeError.test.ts
  • plugins/schedule/test/cronError.test.ts
📚 Learning: 2025-10-23T17:37:37.567Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-10-23T17:37:37.567Z
Learning: Applies to tegg/plugin/*/{test,tests}/**/*.ts : Add Vitest tests for plugins using eggjs/mock; tests will be discovered by the root vitest config

Applied to files:

  • plugins/schedule/test/customType.test.ts
  • plugins/schedule/test/executeError.test.ts
  • plugins/schedule/test/cronError.test.ts
📚 Learning: 2025-10-18T11:59:58.226Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-18T11:59:58.226Z
Learning: Applies to {packages/cookies/test/**/*.test.ts,tools/egg-bin/test/**/*.test.ts} : Mocha test suites (cookies, egg-bin) must use test/**/*.test.ts

Applied to files:

  • plugins/schedule/test/customType.test.ts
  • plugins/schedule/test/executeError.test.ts
  • plugins/schedule/test/cronError.test.ts
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to packages/**/test/**/*.test.ts : Use Node.js built-in assert module for test assertions

Applied to files:

  • plugins/schedule/test/customType.test.ts
  • plugins/schedule/test/executeError.test.ts
  • plugins/schedule/test/cronError.test.ts
📚 Learning: 2025-10-23T17:37:37.567Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-10-23T17:37:37.567Z
Learning: Tests are executed via root vitest.config.ts and should not rely on per-package test scripts

Applied to files:

  • plugins/schedule/test/customType.test.ts
  • plugins/schedule/test/executeError.test.ts
  • plugins/schedule/test/cronError.test.ts
  • package.json
📚 Learning: 2025-09-18T07:56:17.417Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T07:56:17.417Z
Learning: For features affecting HTTP or process orchestration, add regression tests that exercise both the CommonJS and TypeScript example apps

Applied to files:

  • plugins/schedule/test/customType.test.ts
  • plugins/schedule/test/executeError.test.ts
  • plugins/schedule/test/cronError.test.ts
📚 Learning: 2025-10-18T11:59:58.226Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-18T11:59:58.226Z
Learning: Applies to {packages/!(cookies)/**/test/**/*.test.ts,plugins/**/test/**/*.test.ts} : Vitest test files must follow the naming pattern test/**/*.test.ts

Applied to files:

  • plugins/schedule/test/customType.test.ts
  • plugins/schedule/test/cronError.test.ts
📚 Learning: 2025-09-18T07:56:17.417Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T07:56:17.417Z
Learning: Applies to **/test/**/*.test.ts : Mirror the repository test pattern when adding new suites

Applied to files:

  • plugins/schedule/test/customType.test.ts
  • plugins/schedule/test/cronError.test.ts
📚 Learning: 2025-10-18T11:59:58.226Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-18T11:59:58.226Z
Learning: Applies to {packages/**/test/**,plugins/**/test/**} : Use Node.js assert for assertions in Vitest tests

Applied to files:

  • plugins/schedule/test/customType.test.ts
📚 Learning: 2025-10-18T11:59:58.226Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-18T11:59:58.226Z
Learning: Applies to {packages/**/vitest.config.ts,plugins/**/vitest.config.ts} : Packages using Vitest must include a vitest.config.ts and import test functions from vitest

Applied to files:

  • plugins/schedule/test/customType.test.ts
  • plugins/schedule/test/cronError.test.ts
  • package.json
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to packages/**/test/**/*.test.ts : Name test files as test/**/*.test.ts and run them with Vitest

Applied to files:

  • plugins/schedule/test/cronError.test.ts
  • package.json
📚 Learning: 2025-10-18T11:59:58.226Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-18T11:59:58.226Z
Learning: Applies to plugins/*/package.json : Plugins must include scripts: build (tsdown && rimraf dist *.tsbuildinfo && tsc -p tsconfig.build.json), typecheck (tsc --noEmit), lint (oxlint --type-aware), test (vitest run), prepublishOnly (pnpm run build)

Applied to files:

  • package.json
📚 Learning: 2025-10-23T17:37:37.567Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-10-23T17:37:37.567Z
Learning: Applies to tegg/core/*/package.json : Core packages should include scripts: typecheck, clean, build (tsc --noEmit, rimraf, build pipeline)

Applied to files:

  • package.json
📚 Learning: 2025-10-18T11:59:58.226Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-18T11:59:58.226Z
Learning: Applies to {packages/**/package.json,plugins/**/package.json,tools/**/package.json} : Include script "lint:fix" that runs oxlint with --fix (e.g., "pnpm run lint -- --fix")

Applied to files:

  • package.json
📚 Learning: 2025-10-18T11:59:58.226Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-18T11:59:58.226Z
Learning: Applies to {packages/**/package.json,plugins/**/package.json,tools/**/package.json} : Include script "typecheck": "tsc --noEmit" in all packages

Applied to files:

  • package.json
📚 Learning: 2025-09-18T07:56:17.417Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T07:56:17.417Z
Learning: Run pnpm run test locally before submitting changes

Applied to files:

  • package.json
📚 Learning: 2025-10-18T11:59:58.226Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-18T11:59:58.226Z
Learning: Applies to {packages/**/package.json,plugins/**/package.json,tools/**/package.json} : Include script "lint": "oxlint --type-aware" in all packages

Applied to files:

  • package.json
📚 Learning: 2025-09-18T07:56:17.417Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T07:56:17.417Z
Learning: Use pnpm with Node.js ≥ 20.19.0; keep local tool versions aligned with the repository packageManager

Applied to files:

  • package.json
📚 Learning: 2025-10-18T11:59:58.226Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-18T11:59:58.226Z
Learning: Applies to {packages/**/package.json,plugins/**/package.json,tools/**/package.json} : Use catalog: versions for external dependencies defined in pnpm-workspace.yaml

Applied to files:

  • package.json
📚 Learning: 2025-10-23T17:37:37.567Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-10-23T17:37:37.567Z
Learning: Run build, test, and versioning commands from the monorepo root (Egg.js repo), not from tegg/

Applied to files:

  • package.json
📚 Learning: 2025-10-23T17:37:37.567Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-10-23T17:37:37.567Z
Learning: Applies to tegg/{core,plugin,standalone}/*/package.json : Use workspace:* for internal monorepo dependencies and catalog: for external dependencies

Applied to files:

  • package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)
  • GitHub Check: Socket Security: Pull Request Alerts
  • GitHub Check: Test (ubuntu-latest, 24, 4/5)
  • GitHub Check: Test (ubuntu-latest, 22, 3/5)
  • GitHub Check: Test (macos-latest, 22, 2/5)
  • GitHub Check: Test (windows-latest, 24, 3/5)
  • GitHub Check: Test (windows-latest, 24, 4/5)
  • GitHub Check: Test (macos-latest, 22, 4/5)
  • GitHub Check: Test (macos-latest, 24, 4/5)
  • GitHub Check: Test (macos-latest, 24, 3/5)
  • GitHub Check: Test (macos-latest, 22, 3/5)
  • GitHub Check: Test (windows-latest, 22, 1/5)
  • GitHub Check: Test (ubuntu-latest, 24, 3/5)
  • GitHub Check: Test (ubuntu-latest, 22, 1/5)
  • GitHub Check: Test bin (ubuntu-latest, 22, 0/3)
  • GitHub Check: Test bin (windows-latest, 22, 0/3)
  • GitHub Check: Test bin (windows-latest, 22, 1/3)
  • GitHub Check: Test bin (windows-latest, 22, 2/3)
  • GitHub Check: Test bin (ubuntu-latest, 22, 1/3)
  • GitHub Check: Test bin (ubuntu-latest, 22, 2/3)
  • GitHub Check: typecheck
  • GitHub Check: Analyze (javascript-typescript)

"scripts": {
"clean": "pnpm -r run clean && rimraf packages/*/dist plugins/*/dist tools/*/dist examples/*/dist tegg/**/dist **/*/*.tsbuildinfo",
"build": "pnpm -r run build",
"clean": "pnpm -r --parallel run clean && rimraf packages/*/dist plugins/*/dist tools/*/dist examples/*/dist tegg/**/dist **/*/*.tsbuildinfo",
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Expand the tsbuildinfo glob pattern to catch all nested levels.

The pattern **/*/*.tsbuildinfo matches only 2-level-deep files; it will miss root-level and more deeply nested .tsbuildinfo files. Use **/*.tsbuildinfo instead to catch all depths.

-    "clean": "pnpm -r --parallel run clean && rimraf packages/*/dist plugins/*/dist tools/*/dist examples/*/dist tegg/**/dist **/*/*.tsbuildinfo",
+    "clean": "pnpm -r --parallel run clean && rimraf packages/*/dist plugins/*/dist tools/*/dist examples/*/dist tegg/**/dist **/*.tsbuildinfo",
📝 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
"clean": "pnpm -r --parallel run clean && rimraf packages/*/dist plugins/*/dist tools/*/dist examples/*/dist tegg/**/dist **/*/*.tsbuildinfo",
"clean": "pnpm -r --parallel run clean && rimraf packages/*/dist plugins/*/dist tools/*/dist examples/*/dist tegg/**/dist **/*.tsbuildinfo",
🤖 Prompt for AI Agents
In package.json around line 34, the clean script uses the glob
'**/*/*.tsbuildinfo' which only matches two-level-deep .tsbuildinfo files;
change the glob to '**/*.tsbuildinfo' so the clean command removes .tsbuildinfo
files at any nesting depth (including root-level). Update the script value to
use the new glob and keep the rest of the command unchanged.

Comment on lines +35 to +36
"build": "pnpm clean && pnpm -r run build",
"build:parallel": "pnpm clean && pnpm -r --parallel run build",
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify that parallel builds respect inter-package dependencies.

The new build:parallel script uses pnpm -r --parallel run build, which executes all package builds concurrently. This may cause failures if packages have workspace:* dependencies on each other that require sequential build ordering.

Verify that pnpm 10.20.0 respects inter-package dependency ordering even with the --parallel flag, and confirm that all packages can be safely built in parallel. If not, consider removing or documenting this as an advanced/unsupported feature.


🏁 Script executed:

#!/bin/bash
# Description: Verify if pnpm respects workspace:* dependencies with --parallel flag
# Expected result: Check pnpm documentation or test builds to confirm ordering

# Check pnpm version and configuration
pnpm --version

# Search for workspace dependencies that might break parallel builds
ast-grep --pattern $'
{
  "dependencies": {
    $_: "workspace:*",
    $$$
  }
}'

# Check if there are any cross-package build dependencies documented
find . -name "package.json" -type f -not -path "*/node_modules/*" | xargs grep -l "workspace:\*" | head -20

Length of output: 2008


Let me search for information about pnpm's parallel build behavior with workspace dependencies:

Remove or refactor the build:parallel script—it breaks inter-package build ordering.

The --parallel flag disregards topological order and should be avoided for scripts that depend on build artifacts. pnpm's --parallel completely disregards concurrency and topological sorting, running scripts immediately in all packages. Since this repository has inter-package dependencies declared via workspace:* (packages/egg, packages/core, plugins, etc.), running builds in parallel without respecting the dependency graph will cause build failures when dependent packages attempt to build before their dependencies are ready.

Option 1: Remove the build:parallel script entirely—the standard build script correctly respects dependency ordering.

Option 2: If parallel builds are desired, use pnpm -r --workspace-concurrency=N run build (without --parallel) to parallelize within the dependency constraint graph instead.

🤖 Prompt for AI Agents
In package.json around lines 35 to 36, the "build:parallel" script uses pnpm's
--parallel flag which ignores workspace dependency ordering and can break
inter-package builds; either remove the "build:parallel" script entirely so only
the dependency-aware "build" script is used, or replace its behavior by using
pnpm -r --workspace-concurrency=<N> run build (no --parallel) to allow
controlled parallelism while preserving topological order.

Comment on lines +8 to +9
// FIXME: flaky test on Window, Hook timed out in 20000ms
describe.skipIf(process.platform === 'win32')('test/cronError.test.ts', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix typo and consider tracking the root cause.

The comment contains a typo: "Window" should be "Windows". While conditionally skipping flaky tests on Windows is a pragmatic short-term solution, it reduces test coverage on that platform and may hide Windows-specific issues.

Apply this diff to fix the typo:

-// FIXME: flaky test on Window, Hook timed out in 20000ms
+// FIXME: flaky test on Windows, Hook timed out in 20000ms

Consider adding a GitHub issue reference to track investigation of the root cause, e.g., // FIXME(#ISSUE_NUMBER): flaky test on Windows...

📝 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
// FIXME: flaky test on Window, Hook timed out in 20000ms
describe.skipIf(process.platform === 'win32')('test/cronError.test.ts', () => {
// FIXME: flaky test on Windows, Hook timed out in 20000ms
describe.skipIf(process.platform === 'win32')('test/cronError.test.ts', () => {
🤖 Prompt for AI Agents
In plugins/schedule/test/cronError.test.ts around lines 8-9, fix the typo in the
comment and make the FIXME actionable: change "Window" to "Windows" and update
the FIXME to include a GitHub issue reference (e.g. // FIXME(#ISSUE_NUMBER):
flaky test on Windows, Hook timed out in 20000ms), keeping the conditional
skipIf for now; create and reference a new issue to track investigation of the
root cause so the comment links to ongoing work.

Comment on lines +8 to +9
// FIXME: flaky test on Window, Hook timed out in 20000ms
describe.skipIf(process.platform === 'win32')('test/customType.test.ts', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix typo and consider tracking the root cause.

Same issue as in the other schedule tests: "Window" should be "Windows". The consistent pattern across all three schedule tests suggests a systematic Windows flakiness issue that may warrant deeper investigation.

Apply this diff to fix the typo:

-// FIXME: flaky test on Window, Hook timed out in 20000ms
+// FIXME: flaky test on Windows, Hook timed out in 20000ms

Since all three schedule tests exhibit the same Windows timeout behavior, consider consolidating investigation into a single tracked issue and referencing it in all three FIXME comments.

📝 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
// FIXME: flaky test on Window, Hook timed out in 20000ms
describe.skipIf(process.platform === 'win32')('test/customType.test.ts', () => {
// FIXME: flaky test on Windows, Hook timed out in 20000ms
describe.skipIf(process.platform === 'win32')('test/customType.test.ts', () => {
🤖 Prompt for AI Agents
plugins/schedule/test/customType.test.ts lines 8-9: fix the typo in the FIXME
comment by changing "Window" to "Windows" and remove or update the skip reason
accordingly; additionally, create a single tracked issue for the repeated
Windows flakiness and update the FIXME comments in all three schedule tests to
reference that issue ID (or include a TODO link) so the root cause investigation
is consolidated.

Comment on lines +8 to +9
// FIXME: flaky test on Window, Hook timed out in 20000ms
describe.skipIf(process.platform === 'win32')('test/executeError.test.ts', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix typo and consider tracking the root cause.

Same issue as in cronError.test.ts: "Window" should be "Windows". Skipping the test on Windows provides a temporary workaround but leaves the underlying flakiness unaddressed.

Apply this diff to fix the typo:

-// FIXME: flaky test on Window, Hook timed out in 20000ms
+// FIXME: flaky test on Windows, Hook timed out in 20000ms

Consider adding a GitHub issue reference to track investigation of the root cause.

📝 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
// FIXME: flaky test on Window, Hook timed out in 20000ms
describe.skipIf(process.platform === 'win32')('test/executeError.test.ts', () => {
// FIXME: flaky test on Windows, Hook timed out in 20000ms
describe.skipIf(process.platform === 'win32')('test/executeError.test.ts', () => {
🤖 Prompt for AI Agents
In plugins/schedule/test/executeError.test.ts around lines 8-9, fix the typo in
the FIXME comment from "Window" to "Windows" and add a tracking reference to a
GitHub issue for investigating the flakiness; update the comment to read
something like "// FIXME: flaky test on Windows, Hook timed out in 20000ms — see
GH-<issue-number>" (create an issue first and use its number) so the skip
remains but the root cause is tracked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants