Skip to content

[Urgent] fix: include npm config in Docker install layer - #1630

Merged
diegosouzapw merged 2 commits into
cheaper-inference:release/v3.7.1from
rdself:coder/fix-v3.7.1-docker-build
Apr 26, 2026
Merged

diegosouzapw merged 2 commits into
cheaper-inference:release/v3.7.1from
rdself:coder/fix-v3.7.1-docker-build

Conversation

@rdself

@rdself rdself commented Apr 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Set NPM_CONFIG_LEGACY_PEER_DEPS=true inside the Docker builder before npm ci runs.
  • Remove the duplicate postinstallSupport.mjs copy in the dependency install layer.

Impact

  • The Docker install issue prevented the v3.7.0 container image from being built correctly.

Root Cause

The release lockfile relies on npm legacy peer dependency resolution to avoid installing unused @lobehub/icons peer packages. The Docker build did not provide that npm config before npm ci, so npm attempted to resolve the peer dependency tree and failed with lockfile sync errors for @lobehub/ui, antd, lucide-react, and related packages.

Validation

  • git diff --check
  • Reproduced the install failure in a clean temp directory without legacy peer dependency config.
  • Confirmed NPM_CONFIG_LEGACY_PEER_DEPS=true npm ci --ignore-scripts --dry-run --no-audit --no-fund succeeds in a clean temp directory without copying .npmrc.
  • Commit hook checks passed: docs sync and explicit-any budget.

Docker daemon was not available locally, so the full image build is left to CI.

@rdself rdself changed the title fix: include npm config in Docker install layer [紧急] fix: include npm config in Docker install layer Apr 26, 2026
@rdself rdself changed the title [紧急] fix: include npm config in Docker install layer [Urgent] fix: include npm config in Docker install layer Apr 26, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request modifies the Dockerfile to include the .npmrc file and cleans up a duplicate COPY instruction. The review feedback highlights security and portability concerns regarding the inclusion of .npmrc and recommends using an environment variable for npm configuration instead.

Comment thread Dockerfile Outdated
&& rm -rf /var/lib/apt/lists/*

COPY package*.json ./
COPY .npmrc ./

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Copying .npmrc into the Docker image is generally discouraged for two reasons:

  1. Security: If the file contains sensitive information (such as authentication tokens for private registries), it will be baked into the image layers, creating a security risk even if the file is not present in the final stage.
  2. Portability: If .npmrc is not checked into the repository (which is common for local or environment-specific configurations), the docker build command will fail for other developers or in CI environments with a "file not found" error.

Since the stated goal is to enable legacy-peer-deps=true, it is safer and more robust to use an environment variable. This achieves the same result without requiring an external file and ensures the build is self-contained.

ENV NPM_CONFIG_LEGACY_PEER_DEPS=true

@rdself
rdself marked this pull request as ready for review April 26, 2026 18:35
@rdself
rdself requested a review from diegosouzapw as a code owner April 26, 2026 18:35
Copilot AI review requested due to automatic review settings April 26, 2026 18:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes Docker image builds by ensuring npm’s install-time configuration is present in the cached dependency-install layer.

Changes:

  • Copy .npmrc into the builder stage before running npm ci / npm install, so installs respect legacy-peer-deps=true.
  • Remove a duplicate COPY of scripts/postinstallSupport.mjs in the same layer.

@diegosouzapw
diegosouzapw merged commit 1c86cd5 into cheaper-inference:release/v3.7.1 Apr 26, 2026
1 of 2 checks passed
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Thanks @rdself for catching this Docker build issue! The fix has been squash-merged into release/v3.7.1 and will ship in the next release. Great catch on the duplicate COPY line too. We appreciate the contribution! 🎉

diegosouzapw added a commit that referenced this pull request Apr 26, 2026
…cleanup), fix test regression

- PR #1630: Set NPM_CONFIG_LEGACY_PEER_DEPS=true in Dockerfile, remove duplicate COPY
- PR #1631: Hide deprecated gemini-claude-* models from catalogs, redirect to Claude 4.6
- Fix provider-models-route test to match renamed model display name
- Update CHANGELOG.md with both PR entries
diegosouzapw added a commit that referenced this pull request Apr 26, 2026
#1630)

Cherry-pick from PR #1630 by @rdself. Fixes Docker build failures caused by
lockfile peer dependency mismatch during npm ci.
@rdself
rdself deleted the coder/fix-v3.7.1-docker-build branch April 26, 2026 21:04
Poid-ZA pushed a commit to Poid-ZA/OmniRoute that referenced this pull request Aug 5, 2026
cheaper-inference#1630)

Cherry-pick from PR cheaper-inference#1630 by @rdself. Fixes Docker build failures caused by
lockfile peer dependency mismatch during npm ci.
Poid-ZA pushed a commit to Poid-ZA/OmniRoute that referenced this pull request Aug 5, 2026
…erence#1630)

fix: include NPM_CONFIG_LEGACY_PEER_DEPS in Docker builder layer — integrated into release/v3.7.1
Poid-ZA pushed a commit to Poid-ZA/OmniRoute that referenced this pull request Aug 5, 2026
…inference#1631 (Antigravity model cleanup), fix test regression

- PR cheaper-inference#1630: Set NPM_CONFIG_LEGACY_PEER_DEPS=true in Dockerfile, remove duplicate COPY
- PR cheaper-inference#1631: Hide deprecated gemini-claude-* models from catalogs, redirect to Claude 4.6
- Fix provider-models-route test to match renamed model display name
- Update CHANGELOG.md with both PR entries
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
cheaper-inference#1630)

Cherry-pick from PR cheaper-inference#1630 by @rdself. Fixes Docker build failures caused by
lockfile peer dependency mismatch during npm ci.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…erence#1630)

fix: include NPM_CONFIG_LEGACY_PEER_DEPS in Docker builder layer — integrated into release/v3.7.1
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…inference#1631 (Antigravity model cleanup), fix test regression

- PR cheaper-inference#1630: Set NPM_CONFIG_LEGACY_PEER_DEPS=true in Dockerfile, remove duplicate COPY
- PR cheaper-inference#1631: Hide deprecated gemini-claude-* models from catalogs, redirect to Claude 4.6
- Fix provider-models-route test to match renamed model display name
- Update CHANGELOG.md with both PR entries
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants