chore(e2e): pack with utoo pm-pack instead of pnpm#6013
Conversation
Remove the last pnpm dependency from the ecosystem-ci (E2E) workflow by packing workspace tarballs with `ut pm-pack` (utoo >= 1.1) instead of `pnpm -r pack`. utoo 1.1 resolves `workspace:` / `catalog:` protocols, but from npm-native sources (the `workspaces` field and `.utoo.toml`) rather than `pnpm-workspace.yaml`, so generate those on demand and restore the tree after. - scripts/gen-utoo-catalog.mjs: generate .utoo.toml from pnpm-workspace.yaml (catalog: -> [catalog], catalogs.<name> -> [catalogs.<name>]); gitignored. - ecosystem-ci/pack-all.mjs: drop-in for `pnpm -r pack` -- generate .utoo.toml, temporarily inject `workspaces`, `ut pm-pack` each publishable package, move the tgz to the repo root for patch-project.ts, then restore package.json. - e2e-test.yml: install/build use the repo-default utoo (matching ci.yml); the pack step installs utoo@1.1.x into an isolated prefix and runs pack-all.mjs via UT_BIN, since 1.1 is not yet on the `latest` tag setup-utoo installs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
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)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe E2E workflow switches from pnpm-based install/build/pack steps to utoo-based commands. Two new scripts add catalog generation and workspace tarball packing for the E2E packaging flow. Changesutoo-based E2E packing migration
Sequence Diagram(s)sequenceDiagram
participant E2EWorkflow
participant packAll as pack-all.mjs
participant genToml as gen-utoo-catalog.mjs
participant ut as ut pm-pack
E2EWorkflow->>packAll: node ecosystem-ci/pack-all.mjs
packAll->>genToml: generateUtooToml(rootDir)
genToml-->>packAll: .utoo.toml content
loop each workspace package
packAll->>ut: ut pm-pack <packageDir>
ut-->>packAll: package tarball
packAll->>packAll: move .tgz to repo root
end
packAll-->>E2EWorkflow: tarballs ready
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
Dependency limit exceeded — report not shown. This pull request scan exceeded the 10,000-dependency limit applied to this scan, so the results are incomplete and may be inaccurate. To avoid reporting false positives, Socket has not posted a report. Upgrade your plan to raise the dependency limit and get complete reports, or view the partial scan in the dashboard. Socket is always free for open source. If this is a non-commercial open source project, contact us to request a free Team account. |
Deploying egg with
|
| Latest commit: |
4b1e463
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://72bcc92a.egg-cci.pages.dev |
| Branch Preview URL: | https://claude-priceless-lamport-9bd.egg-cci.pages.dev |
![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 scripts to pack workspace packages using ut pm-pack and dynamically generate .utoo.toml from pnpm-workspace.yaml. Feedback highlights two key issues: first, any pre-existing .utoo.toml file is permanently overwritten during packing and should be backed up and restored; second, the direct execution check in gen-utoo-catalog.mjs fails on Windows due to path format differences and should be updated to use import.meta.filename.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const utooTomlPath = path.join(rootDir, '.utoo.toml'); | ||
| const utooTomlExisted = fs.existsSync(utooTomlPath); |
There was a problem hiding this comment.
If .utoo.toml already exists, its original content is overwritten by fs.writeFileSync(utooTomlPath, ...) on line 67, but it is never restored in the finally block. This will permanently overwrite any pre-existing .utoo.toml file a developer might have locally.
We should read and store the original content of .utoo.toml if it exists, and restore it in the finally block, just like we do for package.json.
| const utooTomlPath = path.join(rootDir, '.utoo.toml'); | |
| const utooTomlExisted = fs.existsSync(utooTomlPath); | |
| const utooTomlPath = path.join(rootDir, '.utoo.toml'); | |
| const originalUtooToml = fs.existsSync(utooTomlPath) ? fs.readFileSync(utooTomlPath, 'utf8') : null; |
There was a problem hiding this comment.
Fixed in 5092c0e: back up the original .utoo.toml (originalUtooToml) and restore it in finally, deleting only when it did not pre-exist.
| // Restore package.json; remove generated .utoo.toml unless it pre-existed. | ||
| fs.writeFileSync(pkgJsonPath, originalPkgJson); | ||
| if (!utooTomlExisted && fs.existsSync(utooTomlPath)) fs.rmSync(utooTomlPath); |
There was a problem hiding this comment.
Restore the original .utoo.toml file if it pre-existed, or delete it if it was newly created, to avoid permanently overwriting local developer configurations.
// Restore package.json and .utoo.toml
fs.writeFileSync(pkgJsonPath, originalPkgJson);
if (originalUtooToml !== null) {
fs.writeFileSync(utooTomlPath, originalUtooToml);
} else if (fs.existsSync(utooTomlPath)) {
fs.rmSync(utooTomlPath);
}There was a problem hiding this comment.
Fixed in 5092c0e: the finally block now restores a pre-existing .utoo.toml and only rmSyncs it (with force) when it was newly generated.
| if (import.meta.url === `file://${process.argv[1]}`) { | ||
| main(); | ||
| } |
There was a problem hiding this comment.
On Windows, process.argv[1] contains backslashes (e.g., C:\\path\\to\\script.mjs), whereas import.meta.url is a URL with forward slashes and a file:/// prefix (e.g., file:///C:/path/to/script.mjs). This causes the equality check to fail on Windows, meaning main() will never be executed when running the script directly.
Since the project uses Node.js >= 20.11.0 (as evidenced by the use of import.meta.dirname in pack-all.mjs), you can use import.meta.filename and compare it with path.resolve(process.argv[1]) for a robust, cross-platform check.
| if (import.meta.url === `file://${process.argv[1]}`) { | |
| main(); | |
| } | |
| if (process.argv[1] && import.meta.filename === path.resolve(process.argv[1])) { | |
| main(); | |
| } |
There was a problem hiding this comment.
Fixed in 5092c0e: now uses process.argv[1] && import.meta.filename === path.resolve(process.argv[1]), which is correct on Windows.
Deploying egg-v3 with
|
| Latest commit: |
4b1e463
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://7b1f6dbd.egg-v3.pages.dev |
| Branch Preview URL: | https://claude-priceless-lamport-9bd.egg-v3.pages.dev |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #6013 +/- ##
=======================================
Coverage 81.94% 81.94%
=======================================
Files 677 677
Lines 20651 20651
Branches 4099 4099
=======================================
Hits 16922 16922
Misses 3215 3215
Partials 514 514 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/e2e-test.yml:
- Around line 207-210: The isolated `utoo` install in the e2e workflow is using
a floating `1.1.x` range, which can change CI behavior unexpectedly. Update the
`npm install` step in the workflow job that creates `UT_PREFIX` and runs
`pack-all.mjs` to install `utoo` at the exact `1.1.1` version instead, and keep
future version bumps deliberate.
In `@ecosystem-ci/pack-all.mjs`:
- Around line 62-67: The .utoo.toml handling in pack-all.mjs is clobbering an
existing repository file because the script always writes the generated content
but only deletes it when the file did not exist before. Update the try/finally
flow around utooTomlPath and utooTomlExisted so the original .utoo.toml contents
are preserved and restored after the pm-pack run, rather than leaving the
generated file in place. Use the existing utooTomlPath, utooTomlExisted, and
generateUtooToml logic to either back up the original file before writeFileSync
or read/restore it in the cleanup branch.
🪄 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: d7953efe-e95d-4201-8d2a-d3393dad3e8d
📒 Files selected for processing (3)
.github/workflows/e2e-test.ymlecosystem-ci/pack-all.mjsscripts/gen-utoo-catalog.mjs
- pack-all.mjs: apply publishConfig (exports -> dist) onto each manifest before `ut pm-pack`, since utoo does not apply it. Published tarballs now ship dist exports instead of src, fixing downstream MODULE_NOT_FOUND in the E2E jobs. - pack-all.mjs: back up and restore a pre-existing .utoo.toml; replace existsSync checks with try-based reads to avoid the file-system race CodeQL flagged. - gen-utoo-catalog.mjs: Windows-safe direct-run check via import.meta.filename. - e2e-test.yml: pin the isolated utoo install to 1.1.1. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ecosystem-ci/pack-all.mjs (1)
38-40: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winUse an allowlist for
publishConfigoverrides
This denylist still lets config-only keys likeignoreleak into the packedpackage.json, and future npm-only keys will have the same problem. Since packages here already usepublishConfig.exports,main, andtypesas manifest overrides, copying only known manifest fields is safer.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ecosystem-ci/pack-all.mjs` around lines 38 - 40, The current publishConfig filtering in pack-all.mjs uses a denylist, which can still leak npm-only settings like ignore into the packed manifest. Update the publishConfig override logic to use an allowlist of known manifest fields instead of PUBLISH_CONTROL_KEYS, and only copy supported manifest overrides such as exports, main, and types when building the published package.json.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@ecosystem-ci/pack-all.mjs`:
- Around line 38-40: The current publishConfig filtering in pack-all.mjs uses a
denylist, which can still leak npm-only settings like ignore into the packed
manifest. Update the publishConfig override logic to use an allowlist of known
manifest fields instead of PUBLISH_CONTROL_KEYS, and only copy supported
manifest overrides such as exports, main, and types when building the published
package.json.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f91ad664-ef00-490d-bf85-5a58a327a732
📒 Files selected for processing (3)
.github/workflows/e2e-test.ymlecosystem-ci/pack-all.mjsscripts/gen-utoo-catalog.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/gen-utoo-catalog.mjs
Address review feedback: copy only known manifest fields (exports, main, types, bin, module, ...) from publishConfig instead of denylisting publish-control keys, so config-only keys (e.g. `ignore`) cannot leak into the packed package.json. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Addressed the |
Summary
Remove the last
pnpmdependency from the ecosystem-ci (E2E) workflow by packingworkspace tarballs with utoo's
ut pm-packinstead ofpnpm -r pack.PR #5963 switched the repo from pnpm to utoo but had to keep
pnpmaround inE2E solely for
pnpm -r pack, becauseut pm-packcould not resolve pnpmworkspace:/catalog:protocols in the emitted manifests. utoo >= 1.1 cannow resolve them, so E2E no longer needs pnpm at all.
How it works
utoo 1.1 resolves the protocols, but from npm-native sources rather than
pnpm-workspace.yaml, and does not replicate everynpm packbehavior. The newtooling bridges the gaps during the pack step and restores the tree afterward:
workspace:*is resolved via the npm-styleworkspacesfield in the rootpackage.json→ injected temporarily frompnpm-workspace.yaml.catalog:/catalog:<name>is resolved from.utoo.toml(
[catalog]/[catalogs.<name>]) → generated frompnpm-workspace.yaml.ut pm-packdoes not applypublishConfig, which egg packages use to flipexportsfromsrc/*.tstodist/*.jsat publish time →pack-all.mjsapplies
publishConfigonto each manifest before packing (otherwise thetarballs ship
srcexports and downstream installs fail withMODULE_NOT_FOUND).ut pm-pack <path>writes the tgz into the package dir (no--pack-destination) → moved to the repo root forpatch-project.ts.Files:
scripts/gen-utoo-catalog.mjs— generate.utoo.tomlfrompnpm-workspace.yaml(gitignored; not committed).ecosystem-ci/pack-all.mjs— drop-in replacement forpnpm -r pack..github/workflows/e2e-test.yml— install/build use the repo-default utoo(matching
ci.yml); the pack step installsutoo@1.1.1into an isolatedprefix and runs
pack-all.mjsviaUT_BIN, since 1.1 is not yet on thelatesttag thatsetup-utooinstalls. Once 1.1 is promoted tolatest,the isolated install can be dropped and
ut pm-packcalled directly.Tests
ut run lint,ut run typecheck,ut run fmtcheck,ut run build— pass.vitest run) — pass (3430 passed, 0 failed).pack-all.mjsagainst utoo 1.1.1 (post-build): 79 tarballs, 0 residualworkspace:/catalog:, 0 tarballs exportingsrc/*.ts; named catalog(
packages/router→^1.9.0) andworkspace:*(egg→@eggjs/coreexactversion) resolve;
@eggjs/utils/@eggjs/binexportdist; tree restored.Summary by CodeRabbit
New Features
Bug Fixes