fix: Fix the deployment to viewer-dev caused by missing .npmrc - #6076
Conversation
fix(ci): restore committed .npmrc in Docker build jobs (pnpm migration) The pnpm migration (#6031) made the Dockerfile COPY .npmrc into the builder stage because pnpm needs the workspace config it carries (node-linker=hoisted, link-workspace-packages). But the Docker jobs still ran `rm -f ./.npmrc` immediately before `docker build` -- a leftover from when .npmrc only held npm credentials. NPM_PUBLISH overwrites the tracked .npmrc with a publish auth token and persists it to the workspace, so the Docker jobs deleted the file the Dockerfile requires: ERROR [builder 6/13] COPY package.json pnpm-lock.yaml pnpm-workspace.yaml .npmrc ./ failed to compute cache key: "/.npmrc": not found DEPLOY_MASTER therefore failed at DOCKER_BETA_PUBLISH on every master build after the migration (NPM_PUBLISH still pushed the version bump first, so npm packages shipped but the latest-beta image never did -- leaving viewer-dev.ohif.org stuck on the prior beta). Replace `rm -f ./.npmrc` with `git checkout -- .npmrc` in the four Docker *build* jobs so the committed, token-free workspace-config .npmrc is restored before the build. The two manifest jobs keep `rm` since they do not build from the Dockerfile. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> @
❌ Deploy Preview for ohif-dev failed. Why did it fail? →
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe CircleCI configuration is updated to restore the committed ChangesDocker .npmrc Restoration in CircleCI Jobs
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
✨ 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 |
Merging for testing since this can't be tested without a merge to master
fix(ci): use --no-frozen-lockfile for docs deploy (pnpm migration)
The Build and Deploy Docs workflow ran `pnpm install --frozen-lockfile` and
failed on every master push after the pnpm migration:
[ERR_PNPM_OUTDATED_LOCKFILE] pnpm-lock.yaml is not up to date with
platform/core/package.json
- @ohif/ui (lockfile: workspace:*, manifest: 3.13.0-beta.90)
publish-version.mjs rewrites @ohif/* workspace deps from "workspace:*" to the
concrete release version and commits that bump to master, so the committed
manifests intentionally drift from pnpm-lock.yaml. Every CircleCI job already
passes --no-frozen-lockfile for exactly this reason (pnpm-workspace.yaml sets
frozenLockfile:true as the default); the docs workflow was the lone job still
using frozen and so broke the docs deploy.
Switch the docs install to --no-frozen-lockfile to match the rest of CI.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@
* fix(ci): write npm auth token to ~/.npmrc so publishes authenticate
NPM_PUBLISH wrote the registry auth token to ~/repo/.npmrc (the repo root),
but publish-package.mjs does process.chdir(packageDirectory) and runs
`npm publish` from inside each package (platform/ui, extensions/*, ...). npm
reads the project .npmrc from that package dir and the user .npmrc from
$HOME -- it never walks up to ~/repo/.npmrc -- so every publish failed with
ENEEDAUTH ("need auth ... requires you to be logged in").
publish-package.mjs catches and swallows per-package publish errors, so
NPM_PUBLISH still exited 0 and reported green; the breakage was silent. As a
result no @ohif/* package newer than 3.13.0-beta.89 (the last publish before
the pnpm migration) reached npm -- beta.90 and beta.91 are missing and the
beta dist-tag is stuck at beta.89.
Write the token to ~/.npmrc (npm per-user config, read regardless of cwd)
instead. This also stops clobbering the committed workspace-config .npmrc
(node-linker=hoisted) at the repo root, which the in-job pnpm install/build
relies on. Applied to all three auth steps for consistency.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(docker): copy preinstall.js before pnpm install in builder stage
The builder stage copies only the package.json manifests, runs
`pnpm install --no-frozen-lockfile`, and copies the rest of the source
afterward (for layer caching). But the root package.json defines a
"preinstall" lifecycle script (node preinstall.js) that pnpm runs at the
start of install -- before the source copy -- so the script file was absent
and install aborted:
. preinstall$ node preinstall.js
Error: Cannot find module '/usr/src/app/preinstall.js' (MODULE_NOT_FOUND)
ERROR: process "pnpm install --no-frozen-lockfile" did not complete
This surfaced once the .npmrc COPY fix (#6076) let the build advance past
the earlier COPY failure. preinstall.js is self-contained (it no-ops without
GITHUB_TOKEN and guards the AGENTS.md/CLAUDE.md symlink with existsSync), so
copying just the script into the early manifest layer is sufficient.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(build): declare postcss plugins in platform/app for Docker build
The shared root postcss.config.js (which platform/app re-exports) loads
postcss-import, postcss-preset-env, and cssnano, but none were declared in
platform/app or the root. They only reached node_modules by being hoisted
from platform/docs (postcss-import, postcss-preset-env) and transitively
(cssnano). The Docker image excludes platform/docs via .dockerignore and
libs/@cornerstonejs is not a workspace member, so under pnpm's stricter
node-linker=hoisted the app build failed to resolve the plugins:
Loading PostCSS "postcss-preset-env" plugin failed:
Cannot find module 'postcss-preset-env'
(The accompanying "Can't resolve assets/woff2/latin.woff2" error was a
cascade from the broken PostCSS chain and clears with this fix.)
Declare the three plugins the config explicitly loads as devDependencies of
platform/app, pinned to the versions already resolved by working builds
(postcss-import@14.1.0, postcss-preset-env@7.8.3, cssnano@5.1.15), so the
build no longer depends on incidental hoisting from docs.
The lockfile was regenerated against a workspace:* baseline so the only
@OHIF change is none -- the diff adds just the postcss plugin trees, keeping
specifiers at workspace:* per the repo's convention. Verified by building the
full Docker image locally: rspack compiles with 0 errors and the image
exports successfully.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fix(ci): restore committed .npmrc in Docker build jobs (pnpm migration)
The pnpm migration (#6031) made the Dockerfile COPY .npmrc into the builder stage because pnpm needs the workspace config it carries (node-linker=hoisted, link-workspace-packages). But the Docker jobs still ran
rm -f ./.npmrcimmediately beforedocker build-- a leftover from when .npmrc only held npm credentials. NPM_PUBLISH overwrites the tracked .npmrc with a publish auth token and persists it to the workspace, so the Docker jobs deleted the file the Dockerfile requires:ERROR [builder 6/13] COPY package.json pnpm-lock.yaml pnpm-workspace.yaml .npmrc ./
failed to compute cache key: "/.npmrc": not found
DEPLOY_MASTER therefore failed at DOCKER_BETA_PUBLISH on every master build after the migration (NPM_PUBLISH still pushed the version bump first, so npm packages shipped but the latest-beta image never did -- leaving viewer-dev.ohif.org stuck on the prior beta).
Replace
rm -f ./.npmrcwithgit checkout -- .npmrcin the four Docker build jobs so the committed, token-free workspace-config .npmrc is restored before the build. The two manifest jobs keeprmsince they do not build from the Dockerfile.@
Context
Changes & Results
Testing
Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals.
Tested Environment
Summary by CodeRabbit
Greptile Summary
This PR fixes a broken Docker build pipeline caused by a
rm -f ./.npmrcleftover from before the pnpm migration.NPM_PUBLISHoverwrites.npmrcwith a publish auth token, but the DockerfileCOPYs.npmrcfor the in-imagepnpm install, so deleting it caused every master build to fail after the token was written.rm -f ./.npmrcwithgit checkout -- .npmrcin the four Docker build jobs, restoring the committed pnpm workspace config (node-linker, link-workspace-packages) beforedocker buildruns.DOCKER_MULTIARCH_MANIFEST,DOCKER_BETA_MULTIARCH_MANIFEST) keeprm -f ./.npmrcsince they never invokedocker buildand do not require the file.Confidence Score: 5/5
The change is safe to merge — it restores a broken CI pipeline with a minimal, targeted fix that has no impact on application code.
git checkout -- .npmrccorrectly restores the committed file from the index in both normal and detached-HEAD CI checkouts. The four changed jobs all calldocker buildand genuinely need the file; the two manifest jobs that were intentionally left withrmnever calldocker build. The fix is complete and well-scoped.No files require special attention.
Important Files Changed
rm -f ./.npmrcwithgit checkout -- .npmrcin the four Docker build jobs so the committed pnpm workspace config is restored after NPM_PUBLISH overwrites it; manifest-only jobs correctly retain therm.Reviews (1): Last reviewed commit: "@" | Re-trigger Greptile