-
Notifications
You must be signed in to change notification settings - Fork 22
fix: fix auto publish error #158
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughCI/CD workflows are updated to upgrade pnpm tooling (v2→v4, version 9), introduce version bumping, and add build and cleanup steps. Artifact paths are changed from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
package.json (1)
34-59: Keep scripts metadata consistent + ensure clean doesn’t undermine publish/build expectations.
scripts-info.prepublishOnly(Line 71) is now stale/misleading ifprepublishOnlywas intentionally removed fromscripts.cleannow only removesbuildandtest(Line 58); if the publish pipeline expectsdist/cleanup (or any other dirs), this may leave old artifacts around locally.Also applies to: 60-72
.github/workflows/auto-publish.yml (1)
14-29: Pinactions/checkoutand alignsetup-nodeversions.
actions/checkout@master(Line 15) uses an unpinned reference; pin toactions/checkout@v4(or a commit SHA) for reproducibility and supply-chain safety.- Build job uses
actions/setup-node@v3(Line 25) while publish job usesactions/setup-node@v4(Line 64); align both to the same major version to ensure consistent behavior.- Note: Publish job invokes
setup-node@v4without a precedingcheckoutstep, relying instead on the downloaded artifact. This works but is architecturally inconsistent with the build job.
🧹 Nitpick comments (3)
.github/workflows/auto-publish.yml (1)
46-54: Harden publish auth + reduce noisy/fragile steps.
- Writing
~/.npmrcmanually (Line 93) works, but consider lettingsetup-nodemanage auth (and/or delete~/.npmrcafter publish).ls(Line 49) is just log noise; if you need diagnostics, print targeted paths.Also applies to: 91-97
.github/workflows/dispatch-publish.yml (2)
83-89: Make the “Bump version” step deterministic without relying onjq.
jq(Line 88) may not be present in all runner images long-term; you can print via Node instead (consistent with earlier version read):node -p "require('./package.json').version".
122-127: Prefer setup-node-managed auth and clean up credentials.
Instead of writing~/.npmrc(Line 124), consider using the built-in npm auth handling viaactions/setup-node(and/or remove the file after publish) to reduce credential residue on the runner.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/auto-publish.yml(3 hunks).github/workflows/dispatch-publish.yml(2 hunks)package.json(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: BackEnd UnitTest
package.json
[warning] 1-1: The field "resolutions" was found in package.json. This will not take effect. Configure "resolutions" at the root of the workspace instead.
🪛 GitHub Actions: Playwright E2E Tests
package.json
[error] 190-190: pnpm: duplicated mapping key. Duplicate 'specifier' entry detected (lines 190:9). This indicates conflicting specifier values for the same package (e.g., '^8.1.1' vs '8.1.1').
[warning] 1-1: Resolutions field found in package.json. This will not take effect; configure 'resolutions' at the workspace root instead.
🔇 Additional comments (2)
package.json (1)
2-3: Confirm registry/package rename intent and downstream consumers impact.
Renaming to@opentiny/tiny-toolkit-pro(Line 2) + bumping to1.4.0-beta.0(Line 3) will break installs/imports for consumers still on@opentiny/fluent-editor; please ensure README, release notes, and any internal dependents are updated accordingly..github/workflows/dispatch-publish.yml (1)
41-45: Verify pnpm v9 + lockfile compatibility (CI currently failing).
Given the reportedpnpm: duplicated mapping keyerror in CI, please confirm the updated workflows + pnpm v9 don’t keep failing on the existingpnpm-lock.yaml(likely needs a lockfile fix/regen or conflict resolution).
| - name: Build | ||
| run: pnpm build | ||
| - name: Clean node_modules | ||
| run: | | ||
| pnpm build | ||
| # 删除任何意外混进去的 node_modules | ||
| ls | ||
| rm -rf node_modules | ||
| find template -type d -name node_modules -prune -exec rm -rf {} + | ||
| # 生成待发布 tarball(验证用) | ||
| npm pack --dry-run | ||
| - name: Upload build artifact | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: dist-artifact | ||
| path: dist/ | ||
| path: ./ | ||
| # Publish job |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/auto-publish.ymlRepository: opentiny/tiny-pro
Length of output: 3914
🏁 Script executed:
head -30 package.jsonRepository: opentiny/tiny-pro
Length of output: 676
Avoid uploading the entire repo (path: ./) as an artifact.
Using path: ./ at lines 58 and 72 uploads/downloads the entire repository unnecessarily. Even with node_modules cleaned, this still includes source files, .git history, .github workflows, and other files unneeded for npm publishing. Upload only the minimal publish set: dist/**, package.json, README*, and LICENSE*. For the download step, use a dedicated folder like path: artifact/ to avoid cluttering the working directory.
🤖 Prompt for AI Agents
.github/workflows/auto-publish.yml around lines 44 to 59: the workflow currently
uploads the entire repository with "path: ./", which includes unnecessary files;
change the Upload build artifact step to only include the minimal publish set
(e.g., dist/**, package.json, README*, LICENSE*) and set the path accordingly,
and for the corresponding download step use a dedicated folder (e.g., path:
artifact/) to avoid cluttering the workspace and prevent uploading .git,
.github, source files, and other unrelated files.
| - name: Clean node_modules | ||
| run: | | ||
| # 删除任何意外混进去的 node_modules | ||
| ls | ||
| rm -rf node_modules | ||
| find template -type d -name node_modules -prune -exec rm -rf {} + | ||
| # 生成待发布 tarball(验证用) | ||
| npm pack --dry-run | ||
| - name: Upload build artifact | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: dist-artifact | ||
| path: dist/ | ||
| path: ./ | ||
| # Publish job |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/dispatch-publish.yml | sed -n '85,125p'Repository: opentiny/tiny-pro
Length of output: 1667
🏁 Script executed:
cat package.json | jq '.files // "No files field"'Repository: opentiny/tiny-pro
Length of output: 77
Minimize artifact scope: path: ./ uploads entire repo without a files field restriction.
Uploading the entire repository to dist-artifact (line 103) and downloading it to ./ (line 117) is problematic because package.json lacks a files field. This means npm publish will include all non-ignored files—source code, build config, tests, and workflows—increasing package size and exposing unnecessary implementation details. Instead, either:
- Add a
filesfield topackage.jsonto restrict what npm publishes, or - Upload only the necessary build outputs (dist/, build artifacts) to a minimal artifact directory and download to a dedicated location.
🤖 Prompt for AI Agents
.github/workflows/dispatch-publish.yml around lines 91 to 104: the workflow
currently uploads the entire repository via actions/upload-artifact with path:
./ which causes npm publish to potentially include all repo files; either (A)
narrow the artifact to only build outputs by creating a minimal temp publish
directory (e.g., mkdir -p publish && cp -R dist package.json README.md LICENSE
into publish) and change the upload-artifact path to that directory, and ensure
the corresponding download step restores to a dedicated location, or (B) add a
"files" field to package.json listing only the files/dirs that should be
published (e.g., ["dist","README.md","LICENSE"]) so npm publish will be scoped;
implement one of these two fixes and update the upload/download paths
accordingly.
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.