Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughMigrates CI and workspace tooling from pnpm to utoo/ut, updates package scripts and workspace metadata, adjusts module resolution and build/test tooling in tools/egg-bin, and tightens tests and build configs to reflect new resolution/install behaviors. Changes
Sequence Diagram(s)(Skipped — changes are not best represented by a new multi-component sequential diagram.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Contributor
Summary of ChangesHello, 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 refactors the project's dependency and workspace management. It transitions from using Highlights
Changelog
Ignored Files
Activity
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
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the project's dependency and workspace management by migrating configuration from pnpm-workspace.yaml to a new .utoo.toml file and package.json. My review focuses on the maintainability of the new configuration and the potential impact on the developer workflow. I've suggested sorting the new dependency catalog for better readability and pointed out a potential issue with the removal of the packageManager enforcement that could be addressed with documentation.
Deploying egg-v3 with
|
| Latest commit: |
7083e54
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://025445a0.egg-v3.pages.dev |
| Branch Preview URL: | https://chore-ut-ci.egg-v3.pages.dev |
Deploying egg with
|
| Latest commit: |
7083e54
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://06e337f9.egg-cci.pages.dev |
| Branch Preview URL: | https://chore-ut-ci.egg-cci.pages.dev |
elrrrrrrr
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #5830 +/- ##
==========================================
- Coverage 86.00% 85.49% -0.52%
==========================================
Files 667 660 -7
Lines 18945 18828 -117
Branches 3652 3646 -6
==========================================
- Hits 16294 16097 -197
- Misses 2297 2360 +63
- Partials 354 371 +17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
5 tasks
elrrrrrrr
changed the title
Contributor
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
- Migrate all CI workflows from pnpm to utoo (ut install --from pnpm) - Add scope-limited require.resolve fallback in importResolve: when import.meta.resolve fails for CJS subpaths, fall back to require.resolve but reject results outside the caller's paths scope to prevent workspace-root hoisted packages from being loaded unexpectedly - Add V8 snapshot module loader API (setSnapshotModuleLoader) - Add manifest command to egg-bin for startup manifest management - Refactor buildRequiresExecArgv to base command for reuse - Pin egg-bin tsdown build to local config (avoids root workspace scan) - Bump cnpmcore E2E hash to v4.32.1 (EdgedriverBinary fix) - Add verify-manifest E2E script - Various package version bumps Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
# Conflicts: # .github/workflows/ci.yml # .gitignore # packages/core/src/lifecycle.ts # packages/core/test/snapshot.test.ts # packages/egg/src/lib/egg.ts # packages/egg/src/lib/start.ts # packages/egg/test/__snapshots__/index.test.ts.snap
The previous startsWith path check failed with pnpm because require.resolve follows symlinks and returns .pnpm real paths that don't match the logical paths. Instead, check if the package exists in paths[i]/node_modules/ which works with symlinks. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
node_modules layout varies across package managers (pnpm symlinks, utoo hoisting). Fall back to checking if the package is declared in package.json dependencies/devDependencies/peerDependencies of any provided path entry. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
CI runs in utoo environment where pnpm is not available. Cloudflare Pages build is configured separately and uses pnpm directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Avoids dependency on pnpm or utoo in Cloudflare Pages build environment. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Migrates the monorepo’s primary GitHub Actions CI workflow and root scripts from pnpm-centric commands to utoo (ut), while adapting tooling/tests to work under flat/hoisted node_modules layouts.
Changes:
- Update CI workflow to use
utooland/setup-utooandut install --from pnpm, and switch CI commands tout run …. - Convert root/workspace scripts from
pnpmrecursion/filtering tout run --workspaces/--workspace. - Adjust egg-bin resolution and test bootstrapping for flat-hoisted installs (CJS resolution via
createRequire, and mock autoload guard viapackage.jsondeps).
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/ci.yml |
Switch CI jobs from pnpm setup/install to utoo setup + ut commands. |
package.json |
Replace pnpm-recursive scripts with ut equivalents; adjust site scripts for Cloudflare Pages. |
pnpm-workspace.yaml |
Add utoo to catalog and mark it as onlyBuiltDependencies. |
tsdown.config.ts |
Configure publint packing mode for environments without pnpm. |
tools/egg-bin/src/baseCommand.ts |
Use createRequire().resolve for CJS-only subpath resolution under ESM/flat-hoist. |
tools/egg-bin/src/commands/test.ts |
Guard mock setup loading by checking declared deps in package.json. |
tools/egg-bin/tsdown.config.ts |
Adjust egg-bin build config (entry/external/exports). |
tools/egg-bin/package.json |
Update scripts (add explicit build, remove pretest). |
tools/scripts/package.json |
Update coverage/CI scripts to use ut runner. |
packages/utils/test/import.test.ts |
Loosen error message matching for module resolution differences. |
packages/tsconfig/test/index.test.ts |
Use createRequire + require.resolve for TypeScript bin resolution. |
packages/cluster/test/options.test.ts |
Accept additional flat-hoisted node_modules/egg framework path. |
.oxfmtrc.json |
Ignore root package.json to avoid formatting churn from utoo rewrites. |
.gitignore |
Ignore utoo/Claude-related local files. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
tsdown.config.ts (1)
3-33: Use a typed intermediate config object beforedefineConfig(...).Please align this file with the repo config-typing rule (e.g., assign the config to a typed variable, then export it through
defineConfig).As per coding guidelines
{tsdown.config.ts,vitest.config.ts}: Use typed intermediate variables with appropriate types (UserConfig, UserWorkspaceConfig) for tsdown.config.ts and vitest.config.ts configuration files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tsdown.config.ts` around lines 3 - 33, Create a typed intermediate config object and pass it to defineConfig instead of inlining the object; e.g., import the appropriate config types (UserConfig and UserWorkspaceConfig) from the tsdown typing, declare a const config: UserConfig = { workspace: { /* ... */ } , unused: { /* ... */ }, exports: { /* ... */ }, fixedExtension: false, publint: { /* ... */ }, entry: 'src/**/*.ts', unbundle: true, external: [/^@eggjs\//, 'egg'] } (use UserWorkspaceConfig for the workspace property if applicable) and then export default defineConfig(config). Ensure the variable name (config) replaces the inline object passed to defineConfig and the types match the repo rule.tools/egg-bin/tsdown.config.ts (1)
3-10: Please switch to a typed intermediate tsdown config variable.This keeps the file aligned with monorepo config typing conventions and improves maintainability.
As per coding guidelines
{tsdown.config.ts,vitest.config.ts}: Use typed intermediate variables with appropriate types (UserConfig, UserWorkspaceConfig) for tsdown.config.ts and vitest.config.ts configuration files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/egg-bin/tsdown.config.ts` around lines 3 - 10, Create a typed intermediate config variable instead of inlining the object: import the appropriate type (UserConfig) and declare const tsdownConfig: UserConfig = { entry, unbundle, fixedExtension, external, exports: { devExports: true }, ... } using the existing keys, then pass that variable into defineConfig and export it (export default defineConfig(tsdownConfig)); keep the same properties (entry, unbundle, fixedExtension, external, exports/devExports) and ensure you import the UserConfig type and defineConfig symbol from tsdown so the file follows the monorepo typed-config convention.tools/egg-bin/src/commands/test.ts (1)
306-307: Type the package.json dependency shape instead of usingany.The helper only reads dependency maps, so
Record<string, any>is wider than necessary and drops type safety on the new branch.As per coding guidelines, "Avoid 'any' type in TypeScript; use 'unknown' when type is truly unknown'."Typed helper shape
+interface PackageJsonDependencyFields { + readonly dependencies?: Readonly<Record<string, string>>; + readonly devDependencies?: Readonly<Record<string, string>>; + readonly peerDependencies?: Readonly<Record<string, string>>; +} + -function hasDependency(pkg: Record<string, any>, name: string): boolean { +function hasDependency(pkg: PackageJsonDependencyFields, name: string): boolean { return !!(pkg.dependencies?.[name] || pkg.devDependencies?.[name] || pkg.peerDependencies?.[name]); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/egg-bin/src/commands/test.ts` around lines 306 - 307, The parameter type for hasDependency is too broad (Record<string, any>); define a narrow interface (e.g., PackageDeps or PackageJsonDeps) that types the three dependency maps as optional Record<string, string> (dependencies?, devDependencies?, peerDependencies?) and change the function signature to use that type for pkg; add the new type near hasDependency so callers remain type-safe and avoid using any/unknown.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 209-213: The workspace selector is being passed after the `--` to
the underlying script (e.g., "ut run build -- --workspace ./tools/egg-bin"), so
move the `--workspace` flag so it's consumed by `ut run` itself; update
occurrences like the `ut run build` invocation to use "ut run build --workspace
./tools/egg-bin -- ..." (i.e., place `--workspace` before any `--` separator)
and make the same change for the other affected `ut run` lines (the similar
block around the other occurrence).
In `@package.json`:
- Line 34: Update the package.json script "example:test:all" to use double
quotes for the workspace glob instead of single quotes; change the command
string in the "example:test:all" script so the workspace argument is quoted with
"helloworld-*" (double quotes) to ensure the glob is interpreted correctly on
Windows cmd.exe and cross-platform shells.
In `@tools/egg-bin/src/baseCommand.ts`:
- Around line 217-225: The catch in cjsResolve currently swallows all errors
from createRequire.resolve; change it to only ignore MODULE_NOT_FOUND resolution
errors and rethrow any other exceptions so misconfigurations bubble up. In the
cjsResolve function around createRequire(...).resolve(specifier) inspect the
caught error (e.g., err.code === 'MODULE_NOT_FOUND' or similar) and continue the
loop only for that case, otherwise throw the error; this preserves the fallback
behavior while surfacing broken installs/configuration issues in functions like
cjsResolve and callers that rely on its resolution.
---
Nitpick comments:
In `@tools/egg-bin/src/commands/test.ts`:
- Around line 306-307: The parameter type for hasDependency is too broad
(Record<string, any>); define a narrow interface (e.g., PackageDeps or
PackageJsonDeps) that types the three dependency maps as optional Record<string,
string> (dependencies?, devDependencies?, peerDependencies?) and change the
function signature to use that type for pkg; add the new type near hasDependency
so callers remain type-safe and avoid using any/unknown.
In `@tools/egg-bin/tsdown.config.ts`:
- Around line 3-10: Create a typed intermediate config variable instead of
inlining the object: import the appropriate type (UserConfig) and declare const
tsdownConfig: UserConfig = { entry, unbundle, fixedExtension, external, exports:
{ devExports: true }, ... } using the existing keys, then pass that variable
into defineConfig and export it (export default defineConfig(tsdownConfig));
keep the same properties (entry, unbundle, fixedExtension, external,
exports/devExports) and ensure you import the UserConfig type and defineConfig
symbol from tsdown so the file follows the monorepo typed-config convention.
In `@tsdown.config.ts`:
- Around line 3-33: Create a typed intermediate config object and pass it to
defineConfig instead of inlining the object; e.g., import the appropriate config
types (UserConfig and UserWorkspaceConfig) from the tsdown typing, declare a
const config: UserConfig = { workspace: { /* ... */ } , unused: { /* ... */ },
exports: { /* ... */ }, fixedExtension: false, publint: { /* ... */ }, entry:
'src/**/*.ts', unbundle: true, external: [/^@eggjs\//, 'egg'] } (use
UserWorkspaceConfig for the workspace property if applicable) and then export
default defineConfig(config). Ensure the variable name (config) replaces the
inline object passed to defineConfig and the types match the repo rule.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d12b595f-8a7c-44f1-a16c-8878be92ab2b
📒 Files selected for processing (14)
.github/workflows/ci.yml.gitignore.oxfmtrc.jsonpackage.jsonpackages/cluster/test/options.test.tspackages/tsconfig/test/index.test.tspackages/utils/test/import.test.tspnpm-workspace.yamltools/egg-bin/package.jsontools/egg-bin/src/baseCommand.tstools/egg-bin/src/commands/test.tstools/egg-bin/tsdown.config.tstools/scripts/package.jsontsdown.config.ts
Under flat-hoisting (utoo/npm), every test fork loaded the tegg runner even for fixtures that don't use tegg, costing ~7s per fork (~70s for test-bin suite). The runner was reached via monorepo root hoist rather than a legitimate dependency. Solution: the test harness (coffee.ts) now sets EGG_BIN_SELF_TEST_FIXTURE=1 when spawning egg-bin against its own fixtures. test.ts checks this env var and skips runner auto-detection, avoiding the false-positive load. This keeps the resolve-then-use approach for real projects (including the E2E cnpmcore case where tegg is transitive via egg). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Pinning pack to 'npm' was intended to make main CI portable (utoo env has no pnpm binary), but it forces E2E (pnpm env) to also use npm pack, which takes ~90s per package against pnpm's symlinked node_modules (vs ~2s with pnpm pack). Total E2E build impact was +110s. With auto-detect: - E2E: pnpm-lock.yaml present → pnpm pack (fast, ~13s total) - Main CI: no lockfile → falls back to npm pack (still fast here since utoo's flat node_modules doesn't have the symlink traversal overhead) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Auto-detect picked pnpm pack based on pnpm-workspace.yaml presence, but main CI (utoo env) has no pnpm binary and fails. Pinning to npm works in main CI but causes 10x slowdown in E2E against pnpm's symlinked node_modules. Use env-controlled pack: default 'npm' (keeps main CI portable), E2E workflow sets PUBLINT_PACK=pnpm (restores fast pnpm pack). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
elrrrrrrr
commented
Apr 14, 2026
Drop hasDependency(projectPkg, '@eggjs/mock') in favour of the same EGG_BIN_SELF_TEST_FIXTURE env var that already guards tegg-runner. Two mechanisms doing similar jobs was inconsistent and redundant — the env var is set precisely when we want to opt out (self-test fixtures). For real projects in flat-hoisted monorepos that reach @eggjs/mock via hoist without declaring it, the auto-load still runs. That's acceptable because it was never the target of the guard in the first place; the original motivation was egg-bin's own fixtures, which now use the env. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
fengmk2
approved these changes
Apr 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrate the main CI workflow from pnpm to utoo (
ut). The E2E workflow keeps pnpm for now — it requirescatalog:/workspace:protocol resolution andpublishConfigoverrides during pack whichut pm-packdoes not yet provide.Install speedup
The direct win of this migration:
ut install --from pnpmis 2.2–3.6× faster thanpnpm installacross all test matrix platforms.Every test matrix job re-installs from scratch, so this 15–20s savings per job compounds into the per-job reductions below.
Overall CI wall-clock
On a clean run (no reruns), the full workflow shaves ~1 minute off wall-clock (25m34s → 24m32s, ~4% faster). The critical path is still bounded by Windows Test (~20–24m), so faster jobs elsewhere can't meaningfully shrink wall-clock. Flaky reruns on the critical path can degrade practical times further.
The win of this PR is per-job speedup (especially macOS, where Test jobs drop 6–8 minutes each) and developer-machine ergonomics (
ut installon your laptop is also 3× faster).CI changes
.github/workflows/ci.yml: replacepnpm/action-setup+pnpm installwithutooland/setup-utoo+ut install --from pnpmpackage.json: convert pnpm-specific scripts to ut equivalents (ut run --workspaces,ut run --workspace <name>)site:build/site:devusecd site && npm run build/devso Cloudflare Pages works (its env has neither pnpm nor utoo).gitignore: add.utoo.tomland.claude/.oxfmtrc.json: ignore rootpackage.json(ut rewrites it with npm-styleworkspacesfield)egg-bin fixes
CJS resolution (
baseCommand.ts)ts-node,tsconfig-pathsare CJS packages withoutexports. On flat-hoisted layouts (npm/utoo),importResolve(which usesimport.meta.resolve) fails because the ESM resolver doesn't auto-append.jsfor bare subpaths. Fix: usecreateRequirefor these CJS packages.Self-test fixture opt-out (
test.ts+coffee.ts)Under flat-hoisting, egg-bin's own test fixtures reach
@eggjs/mockand@eggjs/tegg-vitestvia monorepo root hoist even though the fixture projects don't use them. This caused every test fork to load both frameworks unnecessarily (~90s for mock, ~7s/fork for tegg runner → ~70s for the test-bin suite).Solution: the test harness (
coffee.ts) setsEGG_BIN_SELF_TEST_FIXTURE=1when spawning egg-bin against its own fixtures.test.tschecks this env var and skips both mock and tegg-runner auto-detection.For real projects, behaviour is unchanged: both frameworks are resolve-then-use. The tegg runner also falls back to egg-bin's own dirname for E2E scenarios (cnpmcore uses tegg transitively via egg without declaring it directly).
Build perf: publint pack
publintrunspnpm packby default against each of ~80 packages. On main CI (utoo env, no pnpm binary) this fails. On E2E (pnpm env, symlinked node_modules) npm pack is ~10× slower per package —create-eggalone took 85s vs 1s under pnpm.Fix:
tsdown.config.tsreadsPUBLINT_PACKenv var, defaulting tonpm(works in main CI). E2E workflow setsPUBLINT_PACK=pnpmso it stays fast there.Dependency fixes
oxlint-tsgolint: ^0.15.0→^0.18.1@types/content-type,@types/koa-composeto root devDepsOther source adaptations
packages/cluster/test/options.test.tsnode_modules/eggpathpackages/tsconfig/test/index.test.tsrequire.resolveinstead of hardcoded pathtegg/plugin/controller,tegg/plugin/mcp-proxy@ts-expect-errorfor types discoverable under flat hoistingPer-job timing (clean run, no reruns)
Almost every job is faster; the small Test (ubuntu 22) +28s regression is within run-to-run variance.
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes