Skip to content

fix(docker): Add --ignore-scripts to prevent prepare script failure#1213

Merged
yamadashy merged 4 commits intomainfrom
fix/docker-build-prepare-script
Mar 8, 2026
Merged

fix(docker): Add --ignore-scripts to prevent prepare script failure#1213
yamadashy merged 4 commits intomainfrom
fix/docker-build-prepare-script

Conversation

@yamadashy
Copy link
Copy Markdown
Owner

@yamadashy yamadashy commented Mar 8, 2026

Docker builds have been failing since 2025-12-31 (PR #1053) when the prepare script was added to package.json.

The prepare script runs npm run build (rimraf lib && tsc -p tsconfig.build.json), which gets triggered by npm ci --omit=dev in the Dockerfile. Since devDependencies (rimraf, typescript) are not installed with --omit=dev, the build fails with exit code 127 (command not found).

This fix adds --ignore-scripts to the second npm ci --omit=dev call, which is only meant to prune devDependencies — the build output already exists from the prior step.

Checklist

  • Run npm run test
  • Run npm run lint

Open with Devin

The `prepare` script (`npm run build`) added in #1053 causes the Docker
build to fail because `npm ci --omit=dev` triggers it without
devDependencies (`rimraf`, `tsc`) being available. Adding
`--ignore-scripts` skips the unnecessary rebuild since the build output
already exists from the prior step.

Docker builds have been broken since 2025-12-31 (PR #1053).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical issue causing Docker builds to fail. The problem stemmed from a prepare script attempting to run a build step during a npm ci --omit=dev command, which intentionally excludes development dependencies. The fix ensures that this script is bypassed during the final dependency pruning, allowing Docker builds to complete successfully by preventing execution of scripts that rely on uninstalled development tools.

Highlights

  • Docker Build Fix: Resolved Docker build failures that occurred since a prepare script was added to package.json.
  • Root Cause Identified: The prepare script, which runs npm run build, was triggered by npm ci --omit=dev, but devDependencies like rimraf and typescript were not installed, causing command not found errors.
  • Solution Implemented: Added the --ignore-scripts flag to the second npm ci --omit=dev command in the Dockerfile to prevent the prepare script from executing during the dependency pruning phase.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • Dockerfile
    • Added --ignore-scripts to the npm ci --omit=dev command to prevent script execution during dependency pruning.
Activity
  • Author completed npm run test as per the checklist.
  • Author completed npm run lint as per the checklist.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 8, 2026

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 89234e9b-aa19-466d-ac22-f7d17e5e4282

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The Dockerfile's npm install command has been modified to include the --ignore-scripts flag, preventing lifecycle scripts from executing during dependency installation.

Changes

Cohort / File(s) Summary
Build Configuration
Dockerfile
Added --ignore-scripts flag to npm ci command to skip running lifecycle scripts during package installation.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: adding --ignore-scripts to prevent a prepare script failure in Docker builds.
Description check ✅ Passed The description is comprehensive, explaining the root cause, the fix, and why it's safe. The required checklist items are completed.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/docker-build-prepare-script

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.14%. Comparing base (af56918) to head (749392c).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1213   +/-   ##
=======================================
  Coverage   87.14%   87.14%           
=======================================
  Files         115      115           
  Lines        4310     4310           
  Branches      998      998           
=======================================
  Hits         3756     3756           
  Misses        554      554           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

gemini-code-assist[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

Open in Devin Review

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Mar 8, 2026

Code Review

Verdict: Approve

This is a clean, well-targeted fix for the Docker build failure. The change is minimal and correct.

Analysis

The root cause is clearly identified in the PR description: the prepare script (npm run buildrimraf lib && tsc -p tsconfig.build.json) gets triggered by npm ci --omit=dev, but rimraf and typescript are devDependencies that have been excluded by --omit=dev, causing command not found (exit code 127).

The Dockerfile's build pipeline is:

  1. npm ci — install all dependencies (including devDeps)
  2. npm run build — compile TypeScript
  3. npm link — create global symlink
  4. npm ci --omit=dev — prune devDependencies for smaller image ← this re-triggers prepare

Adding --ignore-scripts to step 4 is the correct fix because:

  • The build output already exists from step 2
  • Step 4 is purely a pruning step to reduce image size
  • There's no need to re-run lifecycle scripts during pruning

No concerns found

  • Code quality: Minimal, precise one-line fix — no unnecessary changes
  • Security: --ignore-scripts is actually a security best practice in contexts where scripts don't need to run; appropriate here
  • Performance: Marginally improves Docker build time by skipping an unnecessary build step
  • Edge cases: None — if the build artifact were somehow missing, the repomix --version / repomix --help checks at lines 23-24 would catch it
  • Test coverage: N/A for Dockerfile changes; the existing RUN repomix --version and RUN repomix --help serve as smoke tests

LGTM — ready to merge.


Reviewed by Claude

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages bot commented Mar 8, 2026

Deploying repomix with  Cloudflare Pages  Cloudflare Pages

Latest commit: 749392c
Status: ✅  Deploy successful!
Preview URL: https://2430d0d3.repomix.pages.dev
Branch Preview URL: https://fix-docker-build-prepare-scr.repomix.pages.dev

View logs

Replace `npm ci --omit=dev --ignore-scripts` with `npm prune --production`
which is more efficient (removes packages in-place instead of reinstalling)
and does not trigger lifecycle scripts by default.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
devin-ai-integration[bot]

This comment was marked as resolved.

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Mar 8, 2026

Code Review (Updated)

Verdict: Approve

Note: My previous review incorrectly described the change as adding --ignore-scripts. This updated review reflects the actual diff.

What Changed

The PR replaces npm ci --omit=dev with npm prune --production in the Dockerfile build pipeline:

-    && npm ci --omit=dev \
+    && npm prune --production \

Analysis

This is a better fix than --ignore-scripts would have been:

  • npm prune simply removes extraneous packages from node_modules - it does not trigger lifecycle scripts like prepare, which was the root cause of the failure
  • More efficient: avoids a full dependency resolution and reinstall that npm ci performs; just deletes what is not needed
  • Semantically correct: the intent of this step is remove devDependencies, and npm prune --production expresses that directly

The build pipeline is now:

  1. npm ci - install all dependencies
  2. npm run build - compile TypeScript (needs devDeps)
  3. npm link - create global symlink
  4. npm prune --production - remove devDeps for smaller image
  5. npm cache clean --force - clean cache

Minor Suggestion

Consider using npm prune --omit=dev instead of --production. Node 22 ships with npm 10+, where the --production flag is deprecated in favor of --omit=dev. While --production still works, using --omit=dev would be more future-proof. This is a very minor point - either works correctly today.

PR Title/Description Mismatch

The PR title says Add --ignore-scripts to prevent prepare script failure but the actual change uses npm prune --production. Consider updating the title/description to match the implementation.

No Concerns

  • Correctness: Fix properly addresses the root cause
  • Security: No concerns - npm prune is safe in this context
  • Performance: Slightly faster than npm ci --omit=dev
  • Smoke tests: repomix --version and repomix --help at lines 23-24 validate the build

LGTM - ready to merge.


Reviewed by Claude

Same change as Dockerfile — replace `npm ci --omit=dev --ignore-scripts`
with `npm prune --production` for consistency and efficiency.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Mar 8, 2026

Code Review (Final)

Verdict: Approve

Previous reviews have covered the core analysis. Here are incremental observations on the final state (3 commits):

CI Status

Most checks are passing. The "Build and run" matrix jobs (which exercise the changed CI workflow) are completing successfully on several OS/Node combinations, confirming the fix works.

Minor Nit: CI Step Name

In .github/workflows/ci.yml:308, the step name says "Install only production dependencies" but npm prune --production removes devDependencies rather than installing production ones. Consider updating to match the actual behavior:

- name: Prune development dependencies
  run: npm prune --production

This is purely cosmetic — no functional impact.

Previous Feedback Still Applies

My earlier suggestion about --production vs --omit=dev still stands as a minor future-proofing note (the --production flag is deprecated in npm 10+, which ships with Node 22). Not blocking.

Summary

The PR correctly fixes Docker builds broken since PR #1053 by replacing npm ci --omit=dev (which triggers the prepare script) with npm prune --production (which removes devDeps in-place without lifecycle scripts). Both Dockerfile and ci.yml are updated consistently. LGTM.


Reviewed by Claude

npm 10.x (shipped with Node 22) deprecates the --production flag.
Use --omit=dev instead for both Dockerfile and CI workflow.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Mar 8, 2026

Code Review (Incremental Update)

Verdict: Approve

Previous reviews from Claude covered the core analysis. This is an incremental update on the final 4-commit state.

Changes Since Last Review

The 4th commit (749392c) addressed the --omit=dev suggestion from my previous review — both Dockerfile and ci.yml now use npm prune --omit=dev instead of the deprecated --production flag. Good.

Final Diff Summary

# Dockerfile:17
-    && npm ci --omit=dev \
+    && npm prune --omit=dev \

# .github/workflows/ci.yml:309
-        run: npm ci --omit=dev --ignore-scripts
+        run: npm prune --omit=dev

CI Status

All CI checks pass across the full matrix (ubuntu/windows/macos × Node 20/22/24/25 + Bun). The "Build and run" jobs — which exercise the changed CI workflow step — all completed successfully. Docker build is still in progress.

Remaining Nit (Non-blocking)

ci.yml:308 — Step name is slightly misleading

The step name says Install only production dependencies but npm prune --omit=dev removes devDependencies rather than installing. Consider:

- name: Prune development dependencies
  run: npm prune --omit=dev

Premortem Analysis

Scenario Risk Mitigation
npm prune --omit=dev removes something needed at runtime Low repomix --version and repomix --help smoke tests in both Dockerfile (L23-24) and CI catch this
npm prune behavior differs across npm versions (Node 20 vs 25) Low CI matrix passes on all Node versions (20, 22, 24, 25)
Docker image size regression None npm prune is more efficient than npm ci --omit=dev (modifies in-place vs full reinstall)
PR title/description mismatch with final implementation Cosmetic Title says "Add --ignore-scripts" but final change uses npm prune --omit=dev. Consider updating before merge

LGTM — ready to merge.


Reviewed by Claude

@yamadashy yamadashy merged commit acb744f into main Mar 8, 2026
59 checks passed
@yamadashy yamadashy deleted the fix/docker-build-prepare-script branch March 8, 2026 05:32
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.

1 participant