fix(ci): switch Workers Builds from npm to pnpm (closes #111)#115
fix(ci): switch Workers Builds from npm to pnpm (closes #111)#115chitcommit wants to merge 3 commits into
Conversation
Workers Builds was auto-detecting npm and running `npm clean-install`, but the repo uses pnpm with a committed `pnpm-lock.yaml` and a gitignored `package-lock.json`. The cached npm lockfile drifted from package.json (it still contained `@tailwindcss/vite`, which has been removed from the project), causing every recent build to fail with ERESOLVE peer-dep conflicts against vite 7. The failures masked real CI signal across PRs. Two changes make Workers Builds use pnpm consistently: - Add `"packageManager": "pnpm@10.32.1"` to package.json — Workers Builds reads this field and selects the package manager accordingly, avoiding the npm fallback. - Replace the `npx vite build` build command with `pnpm install --frozen-lockfile && pnpm exec vite build` in both `wrangler.jsonc` and `deploy/system-wrangler.jsonc` so the install step runs against the committed pnpm-lock.yaml regardless of any upstream auto-detection drift. Verified locally: pnpm install --frozen-lockfile (clean) + pnpm exec vite build both succeed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@coderabbitai review Please evaluate:
|
Code Review — PR #115: fix(ci): switch Workers Builds from npm to pnpmSummary: Small, focused build-config fix. Root cause is clear, changes are minimal and correct. Approved with a few minor notes below. OverviewThe PR correctly identifies that Cloudflare Workers Builds was invoking What Looks Good ✅
Minor Issues / Suggestions1. // current
"packageManager": "pnpm@10.32.1"
// corepack best-practice (prevents MITM on pnpm download)
"packageManager": "pnpm@10.32.1+sha512.<hash>"Run 2. Build command runs This is intentional and correct given the platform constraint (Workers Builds was using npm), but it does add install time to every build. If Cloudflare ever adds native pnpm support or if you can configure the package manager at the project level in the Workers Builds UI, this explicit install step can be removed. Worth a comment in the wrangler config or a follow-up ticket so it doesn't get forgotten. 3. Consider // current
"command": "pnpm install --frozen-lockfile && pnpm exec vite build --outDir dist/public"
// alternative — delegates to the npm script
"command": "pnpm install --frozen-lockfile && pnpm run build"The SecurityNo concerns. Changing from Test PlanThe unchecked item ("Watch first Workers Build on this PR") is the critical validation. Everything else looks correct statically. Recommend merging after confirming the first Workers Build on this branch goes green. Verdict: Approve with suggestions — the fix is correct and the suggestions above are non-blocking improvements. Ship it once the live Workers Build passes. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6b4e8cb67
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| "build": { | ||
| "command": "npx vite build --outDir dist/public" | ||
| "command": "pnpm install --frozen-lockfile && pnpm exec vite build --outDir dist/public" |
There was a problem hiding this comment.
Remove CI package install from Wrangler custom build
This change puts pnpm install --frozen-lockfile into build.command, but Cloudflare Workers Builds explicitly does not honor Wrangler build custom-build settings, so this does not reliably fix the Workers Builds npm/pnpm mismatch it targets. At the same time, Wrangler custom builds run during local wrangler dev/wrangler deploy, so this introduces a repeated networked install step for every local deploy/dev path, increasing failure risk and build latency without affecting the CI path you intended to fix.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR updates Cloudflare Workers Builds to use pnpm (matching the repo’s committed pnpm-lock.yaml) to avoid npm lockfile/cache conflicts that were breaking builds.
Changes:
- Add
packageManager: "pnpm@10.32.1"topackage.jsonso build environments select pnpm. - Update Workers
build.commandin both wrangler configs to runpnpm install --frozen-lockfilefollowed bypnpm exec vite build.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| wrangler.jsonc | Switch Workers build command from npx vite build to pnpm-based install + build. |
| package.json | Pin the repository package manager to pnpm. |
| deploy/system-wrangler.jsonc | Mirror the pnpm-based build command change for the deploy config. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "version": "2.0.0", | ||
| "type": "module", | ||
| "license": "MIT", | ||
| "packageManager": "pnpm@10.32.1", |
|
ℹ️ 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)
📝 WalkthroughWalkthroughAdds a repository packageManager field for pnpm, updates Cloudflare Workers build commands to install with Changespnpm Standardization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@deploy/system-wrangler.jsonc`:
- Around line 54-56: The build step's command in the "build" object runs inside
deploy/ so pnpm install fails because package.json is at the repo root; add a
"cwd" property to the same "build" object (alongside "command") and set it to
the repository root (e.g., "..") so the pnpm install and Vite build run from the
correct directory; update the "build" object (the "command" and new "cwd"
properties) accordingly.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 62944f89-26bc-4640-bdbf-74b2ae148197
📒 Files selected for processing (3)
deploy/system-wrangler.jsoncpackage.jsonwrangler.jsonc
`packageManager: "pnpm@10.32.1"` in package.json (added in this PR) conflicts with the hardcoded `version: 10` on the `pnpm/action-setup@v4` step. action-setup@v4 errors with ERR_PNPM_BAD_PM_VERSION when both are present. Remove the workflow's `version:` so packageManager becomes the single source of truth. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@coderabbitai review Please evaluate:
|
Code Review — PR #115: fix(ci): switch Workers Builds from npm to pnpmVerdict: ✅ Approve with minor suggestions — root cause is correctly diagnosed, diff is minimal and precise. OverviewWorkers Builds was invoking What Looks Good ✅
Suggestions (non-blocking)1. // current — version only
"packageManager": "pnpm@10.32.1"
// corepack best-practice — version + SHA hash
"packageManager": "pnpm@10.32.1+sha512.<hash>"Run 2. Build command rebuilds "command": "pnpm install --frozen-lockfile && pnpm exec vite build --outDir dist/public"This is intentional (Workers Builds doesn't natively detect pnpm without 3. The "build": "vite build && esbuild server/dev.ts --platform=node --packages=external --bundle --format=esm --outdir=dist"The current build command only runs SecurityNo concerns. Switching from Test PlanThe unchecked item — "Watch first Workers Build on this PR" — is the critical validation. Merge once the live Workers Build on this branch goes green. |
🧩 Analysis chain
Security Review: Switch Workers Builds from npm to pnpm1. 🔒 Security ImplicationsOverall: Low risk — the changes are confined to build toolchain configuration. The PR modifies only three files: The 2. 🔑 Credential Exposure RiskNo credential exposure detected.
3. 📦 Dependency Supply Chain Concerns
pnpm version audit for
4. 🔄 Breaking API ChangesNone detected.
Summary
The PR is safe to merge. The only actionable suggestion is upgrading to the latest pnpm patch ( 🏁 Script executed: Length of output: 17495
Security Review: PR
|
| CVE | Description | Fixed in | 10.32.1 status |
|---|---|---|---|
CVE-2025-69264 (RCE via git-hosted deps executing lifecycle scripts during pnpm install; affects 10.0.0–10.25.x) |
RCE | 10.26.0 | ✅ Patched |
| CVE-2025-69263 (HTTP/git tarball deps stored without integrity hashes, allowing different code to be served on each install) | Supply chain | 10.26.0 | ✅ Patched |
CVE-2026-23890 (path traversal in bin linking via @-prefixed package names, allowing shims/symlinks outside node_modules/.bin) |
Path traversal | 10.28.1 | ✅ Patched |
| CVE-2026-23889 (path traversal in tarball extraction, files written outside package directory on Windows) | Path traversal | 10.28.1 | ✅ Patched |
pnpm@10.32.1 has no known unpatched CVEs. ✅
--frozen-lockfile is a strong positive
The committed pnpm-lock.yaml will be used as-is, preventing unexpected dependency resolution drift in CI. ✅
pnpm overrides block (positive signal)
package.json already contains security-conscious version overrides for express-rate-limit, @hono/node-server, axios, and rollup — indicating active supply chain hygiene. ✅
onlyBuiltDependencies allowlist is maintained
The pnpm.onlyBuiltDependencies field explicitly lists only better-sqlite3, bufferutil, es5-ext, and esbuild as allowed to run lifecycle scripts. This limits the blast radius of a compromised package. ✅
⚠️ file: dependency will likely break the Workers Build
package.json contains:
"@chittyos/schema": "file:../../CHITTYFOUNDATION/chittyschema"This is a local path dependency pointing two directories above the repo root. When Cloudflare Workers Builds clones this repository into an isolated build environment, that path (../../CHITTYFOUNDATION/chittyschema) will not exist, causing pnpm install --frozen-lockfile to fail immediately. This is a pre-existing issue not introduced by this PR, but it may be the real reason the 0-second build failures described in issue #111 persist even after this fix. This should be the highest-priority follow-up item.
pnpm 11 supply chain improvements not yet available at 10.x
pnpm 11 introduced a default minimum release age of 24 hours, meaning newly published package versions are not eligible for installation until they have existed for at least one day. pnpm 11 also enables blockExoticSubdeps by default, preventing dependencies from being resolved through non-standard sources such as Git repositories or direct tarball URLs. These defaults are not present in 10.x — a longer-term consideration if upgrading.
4. 🔄 Breaking API Changes
None. pnpm exec vite build is a direct functional equivalent of npx vite build. The --outDir dist/public output directory is consistent with the assets.directory in both wrangler config files. No bindings, routes, triggers, environments, or wrangler configuration blocks outside build.command were modified.
Summary
| Category | Finding | Severity |
|---|---|---|
| Security implications | Build-toolchain only; account_id pre-existing exposure |
|
| Credential exposure | No secrets in changed files | ✅ None |
| Supply chain – pnpm CVEs | All known CVEs patched at 10.32.1 | ✅ Low |
Supply chain – file: dependency |
@chittyos/schema: file:../../... will fail in CF Workers Build CI |
🔴 High (pre-existing, blocks fix) |
| Breaking API changes | None | ✅ None |
Recommendation: The PR itself is clean and correct, but the file: local dependency is very likely why CI builds fail in 0 seconds. That should be resolved — either by publishing @chittyos/schema to a registry, using a git dependency, or configuring the Workers Build environment to have access to the monorepo root — before expecting this fix to succeed on CI.
✅ Actions performed
Review triggered.
Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.
|
CI status update after
The Recommend merging once the |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
|
@coderabbitai review Please evaluate:
|
|
Security Review: Switch Workers Builds from npm to pnpm1. 🔒 Security ImplicationsOverall: Low risk — the changes are confined to build toolchain configuration. The PR modifies only three files: The 2. 🔑 Credential Exposure RiskNo credential exposure in the PR changes. The build commands (
3. 📦 Dependency Supply Chain Concernspnpm 10.32.1 CVE Status
Current version landscapeThe latest published pnpm version is
pnpm 11 security defaults not yet available in 10.xpnpm 11 introduces three hardened defaults: a Minimum Release Age of 1,440 minutes (24 hours), blocking of exotic subdependencies by default, and a new Allow Builds model for controlling which packages can execute build scripts during installation — the default posture now favors security over immediacy. These protections are opt-in on 10.x.
|
| Category | Finding | Severity |
|---|---|---|
| Security implications | Build-toolchain only; account_id pre-existing exposure |
|
| Credential exposure | No secrets in changed files | ✅ None |
| Supply chain – pnpm CVEs | All known CVEs patched at 10.32.1 | ✅ Low |
Supply chain – file: dependency |
@chittyos/schema: file:../../... will fail in CF Workers Build CI |
🔴 High (pre-existing, blocks fix) |
| Breaking API changes | None | ✅ None |
The PR changes themselves are clean. The file: local dependency is very likely why CI builds fail in 0 seconds, and that should be resolved via the @chittyos/schema registry migration before expecting the Workers Builds fix to succeed end-to-end.
✅ Actions performed
Review triggered.
Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.
Summary
npm clean-installbut the repo uses pnpm (pnpm-lock.yamlcommitted,package-lock.jsongitignored). A stale cached npm lockfile referenced@tailwindcss/vite@^4.1.3(removed from the project) and conflicted withvite@^7, failing every recent build."packageManager": "pnpm@10.32.1"so Workers Builds picks pnpm.pnpm install --frozen-lockfile && pnpm exec vite buildin bothwrangler.jsoncanddeploy/system-wrangler.jsonc.Closes #111.
Test plan
pnpm install --frozen-lockfilecleanpnpm exec vite build --outDir dist/publicsucceeds🤖 Generated with Claude Code
Summary by CodeRabbit