Open
Conversation
Contributor
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
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 ignored due to path filters (2)
📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughUpdated GitHub Actions workflows to relax pnpm lockfile enforcement (switching Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Deploying egg with
|
| Latest commit: |
80d5fad
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b07b51bb.egg-cci.pages.dev |
| Branch Preview URL: | https://chore-ut-ci-baseline.egg-cci.pages.dev |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the GitHub Actions CI/E2E workflows to use utoo (ut CLI) for installing dependencies and running repo scripts, as a baseline CI configuration change with no source-code modifications.
Changes:
- Replace
pnpm/action-setup+pnpm ...commands withutooland/setup-utoo+ut install/ut run ...across CI jobs. - Add a “Link workspace packages” step in the main test matrix to enforce symlinks for
tegg/*workspaces on non-Windows runners. - Change the E2E packaging step from
pnpm -r packtonode ecosystem-ci/pack-all.mjs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
.github/workflows/e2e-test.yml |
Migrates install/build to ut, and changes packaging step to a Node script. |
.github/workflows/ci.yml |
Migrates CI jobs to ut, removes pnpm cache usage, and adds a symlink enforcement step for tegg/* workspaces. |
.github/workflows/e2e-test.yml
Outdated
| - name: Pack packages into tgz | ||
| run: | | ||
| pnpm -r pack | ||
| node ecosystem-ci/pack-all.mjs |
There was a problem hiding this comment.
node ecosystem-ci/pack-all.mjs is invoked here, but there is no ecosystem-ci/pack-all.mjs in the repository (the only reference found is this workflow). This will cause the E2E workflow to fail at runtime. Either add the referenced script to the repo (and ensure it packs the workspaces as expected) or revert this step back to the previous pnpm -r pack approach (or another existing script).
Suggested change
| node ecosystem-ci/pack-all.mjs | |
| pnpm -r pack |
![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 (1)
.github/workflows/ci.yml (1)
162-163: No caching configured for ut/utoo.The previous pnpm setup included caching via
actions/setup-nodeorpnpm/action-setup. The new configuration doesn't appear to have dependency caching. While acceptable for a baseline CI test, consider adding caching in a follow-up to improve CI performance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 162 - 163, The workflow step that runs "ut install --from pnpm" lacks any dependency caching; update the CI job around the "Install dependencies" step to add caching for pnpm (e.g., use actions/cache with a key based on node-version and lockfile, or use pnpm/action-setup which provides store caching) so subsequent runs reuse the pnpm store or node_modules; reference the existing step name "Install dependencies" and the command "ut install --from pnpm" when adding the cache step and ensure the cache key includes the lockfile (e.g., pnpm-lock.yaml) to bust when dependencies change.
🤖 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 165-180: The symlink loop that iterates over pkg globs (for pkg in
tegg/core/*/package.json tegg/plugin/*/package.json
tegg/standalone/*/package.json) omits workspace entries from
pnpm-workspace.yaml; update the glob list used by the for-loop to also include
packages/*/package.json plugins/*/package.json examples/*/package.json
tools/*/package.json and site/package.json so those workspace packages are
symlinked the same way (preserving the existing name=$(node -e ...)
dir=$(dirname "$pkg") link="node_modules/$name" rm -rf/mkdir -p/ln -sf logic),
ensuring non-Windows runners resolve cross-workspace ESM imports correctly.
- Around line 267-270: Replace the path-based workspace identifier used in the
test step: change the command that currently runs "ut run ci --workspace
tools/scripts" to use the package name instead (use "--workspace
`@eggjs/scripts`") so pnpm workspace resolution works; update the "Run tests" step
where "ut run build" and "ut run ci --workspace tools/scripts" are executed to
call "ut run ci --workspace `@eggjs/scripts`" instead.
In @.github/workflows/e2e-test.yml:
- Around line 158-160: Add a new Node script file named
ecosystem-ci/pack-all.mjs that enumerates all workspace packages (read from the
root package.json "workspaces" or by scanning workspace dirs), runs npm pack (or
equivalent) in each package directory to create the package tarball, and
moves/renames each generated file into the repository root using the format
`${packageName}-${version}.tgz` with any `@` or `/` characters replaced by `-`;
ensure the script uses the package's name and version from its package.json to
construct the sanitized filename and exits non-zero on any pack error so the
workflow step fails clearly—this will produce the exact .tgz artifacts expected
by the following patch-project.ts step.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 162-163: The workflow step that runs "ut install --from pnpm"
lacks any dependency caching; update the CI job around the "Install
dependencies" step to add caching for pnpm (e.g., use actions/cache with a key
based on node-version and lockfile, or use pnpm/action-setup which provides
store caching) so subsequent runs reuse the pnpm store or node_modules;
reference the existing step name "Install dependencies" and the command "ut
install --from pnpm" when adding the cache step and ensure the cache key
includes the lockfile (e.g., pnpm-lock.yaml) to bust when dependencies change.
🪄 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: 43a93e24-d6d4-43cc-ac0c-8fa48c46cbbf
📒 Files selected for processing (2)
.github/workflows/ci.yml.github/workflows/e2e-test.yml
.github/workflows/ci.yml
Outdated
Comment on lines
+165
to
+180
| - name: Link workspace packages | ||
| if: ${{ matrix.os != 'windows-latest' }} | ||
| shell: bash | ||
| run: | | ||
| # ut flat-hoisting may copy workspace packages instead of symlinking; | ||
| # force proper symlinks so cross-workspace ESM imports resolve to source. | ||
| for pkg in tegg/core/*/package.json tegg/plugin/*/package.json tegg/standalone/*/package.json; do | ||
| [ -f "$pkg" ] || continue | ||
| name=$(node -e "console.log(require('./$pkg').name)") | ||
| dir=$(dirname "$pkg") | ||
| link="node_modules/$name" | ||
| rm -rf "$link" | ||
| mkdir -p "$(dirname "$link")" | ||
| ln -sf "../../$dir" "$link" | ||
| echo "$name -> $dir" | ||
| done |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check what workspace packages exist and which have cross-dependencies
echo "=== Workspace packages in pnpm-workspace.yaml ==="
cat pnpm-workspace.yaml
echo -e "\n=== Checking tegg/benchmark packages ==="
fd -t f 'package.json' tegg/benchmark/ 2>/dev/null | while read pkg; do
echo "Package: $pkg"
node -e "const p=require('./$pkg'); console.log(' name:', p.name); console.log(' deps:', Object.keys(p.dependencies || {}).filter(d => d.startsWith('@tegg') || d.startsWith('@eggjs')).join(', ') || 'none')"
done
echo -e "\n=== Checking if other workspace packages need symlinks ==="
fd -t f 'package.json' -d 2 packages/ plugins/ tools/ 2>/dev/null | head -10Repository: eggjs/egg
Length of output: 6087
The symlink step omits other workspace packages defined in pnpm-workspace.yaml.
The symlink script only handles tegg/core/*, tegg/plugin/*, and tegg/standalone/*. However, pnpm-workspace.yaml also defines packages/*, plugins/*, examples/*, tools/*, and site as workspace packages. If any of these have cross-workspace dependencies, their tests may fail silently on non-Windows runners due to incorrect resolution.
Note: tegg/benchmark/* is not defined in pnpm-workspace.yaml, so it does not need inclusion in this symlink step.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml around lines 165 - 180, The symlink loop that
iterates over pkg globs (for pkg in tegg/core/*/package.json
tegg/plugin/*/package.json tegg/standalone/*/package.json) omits workspace
entries from pnpm-workspace.yaml; update the glob list used by the for-loop to
also include packages/*/package.json plugins/*/package.json
examples/*/package.json tools/*/package.json and site/package.json so those
workspace packages are symlinked the same way (preserving the existing
name=$(node -e ...) dir=$(dirname "$pkg") link="node_modules/$name" rm -rf/mkdir
-p/ln -sf logic), ensuring non-Windows runners resolve cross-workspace ESM
imports correctly.
.github/workflows/ci.yml
Outdated
Comment on lines
+267
to
+270
| - name: Run tests | ||
| run: | | ||
| pnpm build | ||
| pnpm run --filter=./tools/scripts ci | ||
| ut run build | ||
| ut run ci --workspace tools/scripts |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the package name in tools/scripts
echo "=== Package name in tools/scripts/package.json ==="
node -e "console.log(require('./tools/scripts/package.json').name)"
# Compare with tools/egg-bin
echo -e "\n=== Package name in tools/egg-bin/package.json ==="
node -e "console.log(require('./tools/egg-bin/package.json').name)"Repository: eggjs/egg
Length of output: 181
🏁 Script executed:
#!/bin/bash
# Search for other usages of "ut run" with workspace to understand the expected format
echo "=== Searching for 'ut run' with workspace arguments ==="
rg 'ut run.*--workspace' .github/workflows/ci.yml -A 1 -B 1
# Check if there's documentation about ut command
echo -e "\n=== Looking for ut documentation or package ==="
fd -name "package.json" -path "*/tools/*" | head -5 | xargs -I {} sh -c 'echo "File: {}" && grep -A2 "name.*ut" {} 2>/dev/null || true'
# Check if tools/scripts is referenced elsewhere as a workspace identifier
echo -e "\n=== Searching for tools/scripts as workspace reference ==="
rg 'tools/scripts|@eggjs/scripts' .github/workflows/ci.yml -B 2 -A 2Repository: eggjs/egg
Length of output: 718
🏁 Script executed:
#!/bin/bash
# Find and examine the ut tool/package
echo "=== Finding ut related files ==="
fd -i 'ut' tools/ -type f -name "package.json" | head -10
# Check tools/scripts package.json for details
echo -e "\n=== Tools/scripts package.json content ==="
cat tools/scripts/package.json
# Check if there's any ut configuration in root or tools
echo -e "\n=== Looking for ut configuration or scripts ==="
rg 'ut.*workspace|workspace.*ut' . --max-count=5
# Check the actual line context from ci.yml to understand the full job
echo -e "\n=== Full context of test-egg-scripts job ==="
sed -n '250,280p' .github/workflows/ci.ymlRepository: eggjs/egg
Length of output: 8101
🏁 Script executed:
#!/bin/bash
# Get the full test-egg-bin job for comparison
echo "=== Full context of test-egg-bin job ==="
sed -n '215,245p' .github/workflows/ci.yml
# Check if utoo/ut documentation exists in the repo
echo -e "\n=== Looking for utoo/ut references ==="
rg 'utoo|ut install|ut run' . --max-count=10 -t yaml -t js -t ts
# Check if there's a ut configuration file
echo -e "\n=== Looking for ut or utoo config ==="
fd -i 'utoo|ut\.config|\.utrc' . 2>/dev/null | head -10Repository: eggjs/egg
Length of output: 1222
🏁 Script executed:
#!/bin/bash
# Check if there's any documentation or comments about ut workspace format
echo "=== Searching for comments or docs about ut workspace ==="
rg 'workspace.*@eggjs|@eggjs.*workspace' . -B 2 -A 2
# Look at the root pnpm-workspace.yaml to understand workspace structure
echo -e "\n=== Root pnpm-workspace.yaml ==="
cat pnpm-workspace.yaml 2>/dev/null || echo "Not found at root"
# Check tools directory structure
echo -e "\n=== Tools directory structure ==="
ls -la tools/ | grep -E 'bin|scripts'
# Search for how pnpm filters are used elsewhere
echo -e "\n=== How pnpm --filter is used in the codebase ==="
rg 'pnpm.*--filter' . -t yaml -t md -t js | head -5Repository: eggjs/egg
Length of output: 50365
🏁 Script executed:
#!/bin/bash
# Look for pnpm-workspace.yaml to understand workspace structure
echo "=== pnpm-workspace.yaml content ==="
cat pnpm-workspace.yaml | head -30
# Check if there are any CI logs or error messages related to workspace resolution
echo -e "\n=== Looking for any workspace-related issues in git history ==="
git log --all --oneline --grep="workspace" 2>/dev/null | head -10
# Search for ut documentation or README
echo -e "\n=== Looking for ut documentation ==="
find . -name "*.md" -o -name "README*" | xargs grep -l "ut run" 2>/dev/null | head -5Repository: eggjs/egg
Length of output: 1508
🏁 Script executed:
#!/bin/bash
# Search for ut command documentation or source
echo "=== Looking for ut tool in package.json or dependencies ==="
grep -r "\"ut\"" . --include="package.json" 2>/dev/null | grep -E "(version|dependency)" | head -5
# Check if there are issues or PRs about workspace resolution
echo -e "\n=== Search for any failing tests related to tools/scripts ==="
rg "tools/scripts|@eggjs/scripts" .github/workflows/ci.yml -B 3 -A 3
# Look for any error messages in workflow artifacts or logs
echo -e "\n=== Checking if there are recent changes to ci.yml ==="
git log --oneline -n 20 .github/workflows/ci.yml 2>/dev/null || echo "Git log not available"
# Try to understand if ut accepts both path and package name formats
echo -e "\n=== Looking for ut tool documentation or help ==="
fd -type f -name "*.md" | xargs grep -l "ut run\|ut install" 2>/dev/null | head -3Repository: eggjs/egg
Length of output: 718
🏁 Script executed:
#!/bin/bash
# Check if there's a ut configuration file in the repo
echo "=== Looking for ut configuration ==="
find . -type f \( -name "ut.config.*" -o -name ".utrc*" -o -name "utoo.config.*" \) 2>/dev/null
# Look at the utooland/setup-utoo action to understand ut better
echo -e "\n=== Checking if there's any documentation about ut in the repo ==="
rg "ut run\|ut install" . --type md | head -10
# Check if both workspace identifier formats have been tested
echo -e "\n=== Full ci.yml context around both ut workspace usages ==="
grep -n "ut run" .github/workflows/ci.yml
# Search for any GitHub issues or discussions about workspace format
echo -e "\n=== Looking for any comments about workspace format in ci.yml ==="
sed -n '225,275p' .github/workflows/ci.yml | grep -E "workspace|tools/scripts|@eggjs" || echo "No relevant comments found"Repository: eggjs/egg
Length of output: 746
Replace path-based workspace identifier with package name on line 270.
Line 270 uses --workspace tools/scripts (a filesystem path) while line 229 uses --workspace @eggjs/bin`` (a package name). The actual package name for tools/scripts is @eggjs/scripts, not a path. Since `ut` relies on pnpm for workspace resolution, it expects the package name format. The path-based identifier will not resolve correctly.
Proposed fix
ut run build
- ut run ci --workspace tools/scripts
+ ut run ci --workspace `@eggjs/scripts`📝 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
| - name: Run tests | |
| run: | | |
| pnpm build | |
| pnpm run --filter=./tools/scripts ci | |
| ut run build | |
| ut run ci --workspace tools/scripts | |
| - name: Run tests | |
| run: | | |
| ut run build | |
| ut run ci --workspace `@eggjs/scripts` |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml around lines 267 - 270, Replace the path-based
workspace identifier used in the test step: change the command that currently
runs "ut run ci --workspace tools/scripts" to use the package name instead (use
"--workspace `@eggjs/scripts`") so pnpm workspace resolution works; update the
"Run tests" step where "ut run build" and "ut run ci --workspace tools/scripts"
are executed to call "ut run ci --workspace `@eggjs/scripts`" instead.
.github/workflows/e2e-test.yml
Outdated
Comment on lines
+158
to
+160
| - name: Pack packages into tgz | ||
| run: | | ||
| pnpm -r pack | ||
| node ecosystem-ci/pack-all.mjs |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the pack-all.mjs script exists
fd -t f 'pack-all.mjs' ecosystem-ci/ 2>/dev/null || echo "pack-all.mjs not found"
# If found, show its contents
cat ecosystem-ci/pack-all.mjs 2>/dev/null | head -50Repository: eggjs/egg
Length of output: 35
🏁 Script executed:
#!/bin/bash
# List all files in ecosystem-ci directory
echo "=== Files in ecosystem-ci directory ==="
find ecosystem-ci -type f 2>/dev/null | head -20
echo ""
echo "=== Search for any 'pack' related files in the repo ==="
fd -t f 'pack' --ignore-case 2>/dev/null | grep -v node_modules | head -20
echo ""
echo "=== Check ecosystem-ci directory structure ==="
ls -la ecosystem-ci/ 2>/dev/null || echo "ecosystem-ci directory not found"Repository: eggjs/egg
Length of output: 1744
🏁 Script executed:
#!/bin/bash
# Check the workflow file context
echo "=== Context around line 158-160 in e2e-test.yml ==="
sed -n '150,170p' .github/workflows/e2e-test.yml
echo ""
echo "=== Search for pack-all references anywhere in the repo ==="
rg 'pack-all' --type-add 'yml:*.yml' --type-add 'ts:*.ts' --type-add 'mjs:*.mjs' 2>/dev/null | head -20
echo ""
echo "=== Search for TODO or FIXME related to packing ==="
rg '(TODO|FIXME).*pack' --type-add 'yml:*.yml' --type-add 'ts:*.ts' --type-add 'mjs:*.mjs' 2>/dev/null | head -10Repository: eggjs/egg
Length of output: 1619
🏁 Script executed:
#!/bin/bash
# Check patch-project.ts to understand what format it expects
echo "=== patch-project.ts content ==="
head -40 ecosystem-ci/patch-project.tsRepository: eggjs/egg
Length of output: 1432
🏁 Script executed:
#!/bin/bash
# Check more of patch-project.ts to see what it expects from tgzPath
echo "=== More of patch-project.ts ==="
sed -n '40,80p' ecosystem-ci/patch-project.ts
echo ""
echo "=== Check git log for recent changes to e2e-test.yml ==="
git log --oneline -20 -- .github/workflows/e2e-test.yml 2>/dev/null | head -5Repository: eggjs/egg
Length of output: 1543
🏁 Script executed:
#!/bin/bash
# Check what the pack-all script should actually do - look for any documentation or previous implementations
echo "=== Search for packing logic in the repo ==="
rg 'pnpm.*pack|pack.*tgz' --type-add 'sh:*.sh' --type-add 'yml:*.yml' 2>/dev/null | head -10
echo ""
echo "=== Check if there's a previous version or any build scripts that handle packing ==="
fd -t f '.*pack.*' --hidden 2>/dev/null | grep -v node_modules | grep -v .git | head -20Repository: eggjs/egg
Length of output: 2379
🏁 Script executed:
#!/bin/bash
# Check the recent commit that changed this to understand the context
echo "=== Check the specific commit that introduced this change ==="
git show bc3db213:.github/workflows/e2e-test.yml 2>/dev/null | sed -n '150,170p'
echo ""
echo "=== Check if pack-all.mjs is mentioned in any PR or issue files ==="
rg 'pack-all' . 2>/dev/null || echo "No mentions found"Repository: eggjs/egg
Length of output: 883
🏁 Script executed:
#!/bin/bash
# Check what's in the root directory to see if tgz files should go there
echo "=== Check if there are any tgz files or build artifacts ==="
ls -la *.tgz 2>/dev/null || echo "No .tgz files found in root"
echo ""
echo "=== Check pnpm pack behavior to understand what pack-all should do ==="
rg -A 5 'pnpm.*pack|pack.*command' . --type-add 'json:*.json' --type-add 'ts:*.ts' 2>/dev/null | grep -v node_modules | head -15Repository: eggjs/egg
Length of output: 1140
Create the missing ecosystem-ci/pack-all.mjs script.
The workflow references ecosystem-ci/pack-all.mjs which does not exist. This script must generate .tgz files for all workspace packages in the repository root with naming format ${packageName}-${version}.tgz (with @ and / replaced by -), as expected by the subsequent patch-project.ts step. Without this script, the workflow fails at the pack step and e2e tests cannot run.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/e2e-test.yml around lines 158 - 160, Add a new Node script
file named ecosystem-ci/pack-all.mjs that enumerates all workspace packages
(read from the root package.json "workspaces" or by scanning workspace dirs),
runs npm pack (or equivalent) in each package directory to create the package
tarball, and moves/renames each generated file into the repository root using
the format `${packageName}-${version}.tgz` with any `@` or `/` characters
replaced by `-`; ensure the script uses the package's name and version from its
package.json to construct the sanitized filename and exits non-zero on any pack
error so the workflow step fails clearly—this will produce the exact .tgz
artifacts expected by the following patch-project.ts step.
Deploying egg-v3 with
|
| Latest commit: |
80d5fad
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://bba716dd.egg-v3.pages.dev |
| Branch Preview URL: | https://chore-ut-ci-baseline.egg-v3.pages.dev |
Baseline test: verify that the project CI passes with pnpm but without a lockfile, to isolate dependency-upgrade issues from ut-migration issues. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
elrrrrrrr
force-pushed
the
chore-ut-ci-baseline
branch
from
bc3db21 to
80d5fad
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
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.
Baseline: next branch code + ut CI config, no source modifications.
Summary by CodeRabbit