cog-build-deploy: npm install + build before deploy (fixes Worker crash)#342
cog-build-deploy: npm install + build before deploy (fixes Worker crash)#342FMXExpress wants to merge 1 commit into
Conversation
User hit "Worker crashed" deploying a Vite + TypeScript + Workers-Sites
project. Root cause: `wrangler deploy` bundles the entry point with
esbuild but does NOT install node_modules or run the project's build
script. So anything past a zero-dependency single-file Worker breaks:
- a dep-using entry fails at bundle time -> "Could not resolve <pkg>"
- a Workers-Sites project serving a built dist/ deploys an empty
site -> the Worker throws at runtime (the "Worker crashed" the
user saw when hitting the URL)
(Also answers the user's other question: wrangler_project_path is NOT
required -- empty = auto-detect, which worked in their run.)
Fix: _deploy_to_cloudflare now, when the located project dir has a
package.json, runs `npm install --no-audit --no-fund` then (only if
package.json declares a "build" script) `npm run build`, before
`wrangler deploy --temporary`. New input install_deps_before_deploy
(default True) gates it; set False for a hand-written single-file
Worker that needs nothing.
Every failure mode returns a placeholder string (no prediction crash):
(deploy failed: npm install exit N)\n<tail>
(deploy failed: npm run build exit N)\n<tail>
(deploy failed: npm install timed out after 10 min)
npm install/build run with the same per-prediction HOME + scrubbed
CF auth env as wrangler. node_modules/dist land in the live workspace
AFTER pasclaw build already wrote workspace_out.zip, so they don't
bloat the returned archive.
Validated against the REAL feature on Node 22 / wrangler 4.103.0:
- Hono + TS worker, raw `wrangler deploy --temporary`:
✘ Could not resolve "hono" (reproduced the failure)
- after npm install + npm run build:
Deployed -> https://pasclaw-vtest-9f3a2.utopian-howler.workers.dev
curl / -> <h1>hi</h1>
curl /api/hello -> {"hello":"world"} (both routes live)
- orchestration unit test with stub npm/wrangler: install -> build
-> deploy ordering, install_deps=False skips npm, npm-install
failure returns the placeholder and never reaches wrangler.
Also learned + noted: reusing a Worker NAME across rapid temp-account
deploys can hit subdomain-provisioning errors (10007/10013); a fresh
name deploys clean. Each cog prediction is a fresh temp account so
this is unlikely in practice, but worth recording.
README: documented install_deps_before_deploy, the npm failure
placeholders, the dependencies+build behavior, and that
wrangler_project_path is not required.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TBcLtmpj7dqA5tyFbGnQon
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7caf32fcea
ℹ️ 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".
| pkg_json = os.path.join(proj, "package.json") | ||
| if install_deps and os.path.isfile(pkg_json): |
There was a problem hiding this comment.
Install from the package root for nested Wrangler configs
When the Wrangler config lives below the JavaScript package root (for example workspace/worker/wrangler.toml with workspace/package.json holding the dependencies and build script), this guard skips the new install/build step entirely because there is no package.json in the config directory. I checked npm's prefix resolution from a child directory and it finds the parent package root, but this code never invokes npm in that case, so dependency-using nested Worker projects still go straight to wrangler deploy without installed deps and can reproduce the crash this change is meant to prevent.
Useful? React with 👍 / 👎.
Answering your two questions
Is
wrangler_project_pathrequired? No. Empty = auto-detect, and your log shows it worked — it found a wrangler config and ran the deploy in.../home/workspace. You don't need to set it.Why did the Worker crash? Your prompt asked for a Vite + TypeScript + npm-scripts + Workers-Sites project, but the cog ran
wrangler deploy --temporarystraight in the project dir without ever doingnpm installornpm run build.wrangler deploybundles the entry point with esbuild but does not installnode_modulesor run build scripts. So:Could not resolve "<pkg>", ordist/deploys an empty site → the Worker throws at runtime = the "crash" you saw when you hit the URL.My earlier validation only proved a zero-dependency single-file Worker works, which is why this slipped through.
Fix
_deploy_to_cloudflarenow, when the project dir has apackage.json, runsnpm install --no-audit --no-fundthen (only if abuildscript is declared)npm run build, beforewrangler deploy --temporary. New inputinstall_deps_before_deploy(defaultTrue) gates it.Validated against the real feature (Node 22 / wrangler 4.103.0)
Reproduced the failure and confirmed the fix with a Hono + TypeScript Worker:
Plus a stub-based orchestration unit test: install → build → deploy ordering,
install_deps=Falseskips npm, and annpm installfailure returns a placeholder without ever reaching wrangler (no prediction crash).Failure handling
Every new failure mode returns a placeholder (prediction never crashes):
(deploy failed: npm install exit N)\n<tail>(deploy failed: npm run build exit N)\n<tail>(deploy failed: npm install timed out after 10 min)node_modules/distare created in the live workspace afterpasclaw buildalready wroteworkspace_out.zip, so they don't bloat the returned archive. npm runs with the same per-prediction HOME + scrubbed Cloudflare auth as wrangler.Also recorded
Reusing a Worker name across rapid temp-account deploys can hit subdomain-provisioning errors (
10007/10013); a fresh name deploys clean. Each cog prediction is a fresh temp account so this is unlikely in practice, but noted.🤖 Generated with Claude Code
https://claude.ai/code/session_01TBcLtmpj7dqA5tyFbGnQon
Generated by Claude Code