Skip to content

fix: remove redundant global CLI installations from Dockerfile#26

Open
kylewanginchina wants to merge 2 commits intomainfrom
claude/issue-25-20260302-1520
Open

fix: remove redundant global CLI installations from Dockerfile#26
kylewanginchina wants to merge 2 commits intomainfrom
claude/issue-25-20260302-1520

Conversation

@kylewanginchina
Copy link
Owner

@kylewanginchina kylewanginchina commented Mar 2, 2026

Closes #25

Summary

  • Removed redundant npm install -g @anthropic-ai/claude-code @openai/codex from Dockerfile
  • Both SDKs bundle their CLI binaries internally
  • Updated CLAUDE.md, README.md, and health-check script

Generated with Claude Code

Summary by CodeRabbit

  • Documentation

    • Clarified SDK vs CLI handling: Codex CLI is bundled; Claude Code CLI must be installed separately. Updated README, execution flow, and troubleshooting messages to reflect SDK-oriented wording.
  • Chores

    • Adjusted container setup to remove broad global CLI installs and reorder/limit global installation to the required CLI.
    • Updated health check messaging to verify the required CLI is present in PATH.

The SDKs bundle their respective CLI binaries internally:
- @anthropic-ai/claude-agent-sdk bundles the Claude Code binary
- @openai/codex-sdk includes @openai/codex as a dependency

Removed `npm install -g @anthropic-ai/claude-code @openai/codex` from
Dockerfile and updated documentation (CLAUDE.md, README.md) and
health-check script to reflect that no separate CLI installation is needed.

Closes #25

Co-authored-by: Kai WANG <kylewanginchina@users.noreply.github.com>
@coderabbitai
Copy link

coderabbitai bot commented Mar 2, 2026

📝 Walkthrough

Walkthrough

Documentation and build scripts were updated to clarify that the Claude Code CLI must be installed separately (not bundled by the Claude Agent SDK), while the Codex CLI is included as a dependency. Docker and health-check logic were adjusted to reflect the changed installation timing and user-facing messages.

Changes

Cohort / File(s) Summary
Claude SDK / Docs
CLAUDE.md, README.md
Clarified that the Claude Code CLI is not bundled by the Claude Agent SDK and must be installed globally; updated guidance, troubleshooting, and execution wording to reflect separate installation requirement.
Codex SDK / Docs
CLAUDE.md, README.md
Noted that the Codex CLI is included as a dependency and that Codex config can be auto-generated from env vars at container startup (example snippet added).
Docker build
Dockerfile
Removed earlier global install of both CLIs; reordered to install only @anthropic-ai/claude-code globally later (after prune), reflecting narrowed global installation scope and timing change.
Runtime health check
scripts/health-check.sh
Adjusted Claude CLI path/version checks and messages to state the CLI is required by @anthropic-ai/claude-agent-sdk; made version probe non-blocking with an informational fallback message.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through docs and Docker too,
Claimed what’s bundled and what’s not true.
Claude needs install, Codex comes along,
I twitched my nose and wrote this song. 🥕

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning Multiple changes appear out of scope: the commit contradicts the PR title by restoring Claude Code CLI installation, making health-check version probes non-blocking, and broadly updating documentation beyond CLI installation changes. Clarify the intent: either remove redundant installations (as stated in title) or restore them. Separate non-bundling documentation updates from CLI installation logic changes.
Linked Issues check ❓ Inconclusive The linked issue #25 lacks specific coding requirements, making it impossible to validate whether code changes meet the stated objectives. Provide detailed coding requirements in the linked issue to enable proper compliance validation.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main change: removing redundant global CLI installations from the Dockerfile, which aligns with the primary objective stated in the PR description.
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 docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch claude/issue-25-20260302-1520

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@CLAUDE.md`:
- Around line 135-143: The CLAUDE.md currently claims "The SDK bundles the
Claude Code binary internally (no separate CLI installation needed)" which is
incorrect; update that sentence to state that the Claude Agent SDK (TypeScript
`@anthropic-ai/claude-agent-sdk`) does NOT bundle the Claude Code CLI and requires
a separate installation (e.g., npm install -g `@anthropic-ai/claude-code` or
adding that global install step in the Dockerfile) and leave the OpenAI Codex
SDK wording as-is noting it bundles its CLI; ensure you reference the "Claude
Agent SDK" and the phrase about bundling the "Claude Code binary" when making
the edits.

In `@scripts/health-check.sh`:
- Around line 44-50: The optional Claude probe currently runs claude --version
and can abort the script under set -e; make it non-blocking by executing the
version probe in a non-fatal manner (e.g., run claude --version in a context
that ignores failures such as adding "|| true" to the call or temporarily
disabling errexit around the probe using set +e / set -e) while keeping the
existing command -v claude check and the informational echo; update the
health-check.sh block that uses command -v claude and claude --version so any
runtime/permission error from claude --version does not cause the script to
exit.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ee4329b and 8f08250.

📒 Files selected for processing (4)
  • CLAUDE.md
  • Dockerfile
  • README.md
  • scripts/health-check.sh
💤 Files with no reviewable changes (1)
  • Dockerfile

@kylewanginchina
Copy link
Owner Author

@claude review一下当前代码

@claude
Copy link
Contributor

claude bot commented Mar 3, 2026

Claude finished @kylewanginchina's task in 7m 10s —— View job


Code Review for PR #26

  • Read PR diff and changed files
  • Review CLAUDE.md changes
  • Review Dockerfile changes
  • Review README.md changes
  • Review scripts/health-check.sh changes
  • Post review summary

Overall Assessment

The PR's intent is good — removing redundant global CLI installations simplifies the Dockerfile. However, there are two issues that need to be addressed before merging, one of which is critical and could cause runtime failures in production.


🔴 Critical: Claude Agent SDK does NOT bundle the Claude Code CLI

Files affected: CLAUDE.md:135, CLAUDE.md:142, CLAUDE.md:183, README.md:291-292, Dockerfile

The PR claims "The SDK bundles the Claude Code binary internally (no separate CLI installation needed)" and "Both SDKs handle their respective CLI binaries internally." This is incorrect for the Claude Agent SDK.

Evidence from package-lock.json:

SDK Dependencies Bundles CLI?
@anthropic-ai/claude-agent-sdk (v0.2.47) Only optional @img/sharp-* packages + zod peer dep No — no @anthropic-ai/claude-code dependency
@openai/codex-sdk (v0.104.0) "@openai/codex": "0.104.0" Yes — directly depends on the CLI package

The TypeScript @anthropic-ai/claude-agent-sdk requires the Claude Code CLI (@anthropic-ai/claude-code) to be installed separately — it is not bundled. The query() function spawns the claude binary which must be available in PATH.

Impact: Removing npm install -g @anthropic-ai/claude-code from the Dockerfile means the Docker container will not have the Claude Code CLI, causing runtime failures when the service tries to execute Claude queries.

Recommended fix: Restore the Claude Code CLI installation in the Dockerfile while keeping the Codex removal:

# Install Claude Code CLI globally (required by claude-agent-sdk which does not bundle it)
RUN npm install -g @anthropic-ai/claude-code

And update documentation to differentiate the two SDKs:

  • Claude Agent SDK: requires separate CLI installation
  • OpenAI Codex SDK: bundles CLI internally (no separate install needed)

Fix this →


🟠 Major: scripts/health-check.shclaude --version can abort the script

File: scripts/health-check.sh:47

The script uses set -e (line 5), which causes any failing command to terminate the script. The claude --version call on line 47 is inside an "optional" block, but if command -v claude succeeds (binary found in PATH) yet claude --version fails for any reason (permissions, corrupted binary, missing runtime deps), it will abort the entire health check.

# Current code (line 44-50):
if command -v claude &> /dev/null; then
    echo "✅ Claude CLI is available in PATH"
    claude --version          # ← This can abort the script under set -e
else
    echo "ℹ️  Claude CLI not found in PATH (this is normal — the SDK bundles the binary internally)"
fi

Fix: Add || true to make the version probe non-fatal:

claude --version || echo "ℹ️  Claude CLI present but version probe failed (non-blocking)"

Fix this →


✅ Good Changes

  • README.md: The documentation updates (SDK terminology, permission bypass mode wording, workflow description) are accurate and well-written.
  • Dockerfile: Removing @openai/codex from global install is correct since @openai/codex-sdk bundles it.
  • CLAUDE.md: The Codex SDK description and troubleshooting updates are fine (only the Claude SDK claims need correction).

Summary

Issue Severity File(s)
Claude Agent SDK does NOT bundle CLI — removing global install will break production 🔴 Critical Dockerfile, CLAUDE.md, README.md
claude --version can abort health-check script under set -e 🟠 Major scripts/health-check.sh

The Codex-related changes are all correct and can stay as-is. The Claude-related changes need revision to reflect that @anthropic-ai/claude-agent-sdk requires a separate CLI installation.


|

…heck

- Claude Agent SDK does NOT bundle the CLI binary; restore global install
  step in Dockerfile and update documentation accordingly
- Make claude --version probe non-blocking in health-check.sh (set -e safe)

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

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
README.md (1)

291-293: Add a parallel Codex troubleshooting entry for provider parity.

This updated section helps for Claude failures; adding a matching Codex failure/remediation item would keep the docs aligned with dual-provider support.

Based on learnings: Support both Claude Agent SDK and OpenAI Codex SDK as AI providers with user specification via mentions: claude (default), codex, or claude[model=xxx] / codex[model=xxx].

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` around lines 291 - 293, Add a parallel "Codex execution failed"
troubleshooting bullet mirroring the Claude entry: include steps to ensure the
OpenAI/Codex CLI or SDK is installed (e.g., install the OpenAI Codex client or
CLI used by this project) and that Docker images are built with all npm
dependencies; also add a short note about provider-selection syntax supported by
the project — "claude" (default), "codex", and the model-specific forms
"claude[model=xxx]" / "codex[model=xxx]" — so readers know how to force provider
parity when diagnosing failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Dockerfile`:
- Around line 26-28: The Dockerfile currently installs the Claude Code CLI
globally via the npm package `@anthropic-ai/claude-code` but does not install the
Codex CLI required by `@openai/codex-sdk`; add a global npm install for the Codex
CLI immediately after the existing Claude install so the container supports
Codex provider as well (ensure you add an npm -g install for the `@openai/codex`
package in the same Dockerfile section where `@anthropic-ai/claude-code` is
installed).
- Around line 26-27: The Dockerfile currently installs the Claude Code CLI
without pinning and never installs the Codex CLI required for the created .codex
directory; update the RUN that installs `@anthropic-ai/claude-code` to pin a
specific version (e.g., replace RUN npm install -g `@anthropic-ai/claude-code`
with a versioned install using an ARG like CLAUDE_CLI_VERSION and RUN npm
install -g `@anthropic-ai/claude-code`@$CLAUDE_CLI_VERSION), and add a global
installation for the Codex CLI (introduce CODEX_CLI_VERSION ARG and install
codex@$CODEX_CLI_VERSION or the correct package name for your Codex CLI) in the
Dockerfile so both CLIs are installed at pinned versions before any use of the
.codex directory.

---

Nitpick comments:
In `@README.md`:
- Around line 291-293: Add a parallel "Codex execution failed" troubleshooting
bullet mirroring the Claude entry: include steps to ensure the OpenAI/Codex CLI
or SDK is installed (e.g., install the OpenAI Codex client or CLI used by this
project) and that Docker images are built with all npm dependencies; also add a
short note about provider-selection syntax supported by the project — "claude"
(default), "codex", and the model-specific forms "claude[model=xxx]" /
"codex[model=xxx]" — so readers know how to force provider parity when
diagnosing failures.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8f08250 and 0732475.

📒 Files selected for processing (4)
  • CLAUDE.md
  • Dockerfile
  • README.md
  • scripts/health-check.sh
🚧 Files skipped from review as they are similar to previous changes (1)
  • CLAUDE.md

Comment on lines +26 to +27
# Install Claude Code CLI globally (required by @anthropic-ai/claude-agent-sdk)
RUN npm install -g @anthropic-ai/claude-code
Copy link

@coderabbitai coderabbitai bot Mar 3, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the Dockerfile around lines 26-27
head -35 Dockerfile | tail -15 | cat -n

Repository: kylewanginchina/gitlab-claude-webhook

Length of output: 546


🏁 Script executed:

#!/bin/bash
# Search for all claude and codex references in Dockerfile
rg -n -i 'claude|codex' Dockerfile

Repository: kylewanginchina/gitlab-claude-webhook

Length of output: 601


🏁 Script executed:

#!/bin/bash
# Check full Dockerfile context around the npm installs
cat -n Dockerfile | grep -A 5 -B 5 'npm install'

Repository: kylewanginchina/gitlab-claude-webhook

Length of output: 433


Address two critical deployment issues: pin Claude CLI version and add missing Codex CLI installation.

Line 27 installs Claude Code CLI without version pinning, making Docker builds non-reproducible. Additionally, the Dockerfile creates a .codex directory (line 36-38) but never installs the Codex CLI globally, which is required for Docker deployments.

Suggested patch
 # Install Claude Code CLI globally (required by `@anthropic-ai/claude-agent-sdk`)
-RUN npm install -g `@anthropic-ai/claude-code`
+RUN npm install -g `@anthropic-ai/claude-code`@1.0.0 && \
+    npm install -g `@anthropic-ai/codex`@1.0.0
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Dockerfile` around lines 26 - 27, The Dockerfile currently installs the
Claude Code CLI without pinning and never installs the Codex CLI required for
the created .codex directory; update the RUN that installs
`@anthropic-ai/claude-code` to pin a specific version (e.g., replace RUN npm
install -g `@anthropic-ai/claude-code` with a versioned install using an ARG like
CLAUDE_CLI_VERSION and RUN npm install -g
`@anthropic-ai/claude-code`@$CLAUDE_CLI_VERSION), and add a global installation
for the Codex CLI (introduce CODEX_CLI_VERSION ARG and install
codex@$CODEX_CLI_VERSION or the correct package name for your Codex CLI) in the
Dockerfile so both CLIs are installed at pinned versions before any use of the
.codex directory.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Thanks for the suggestions!

Re: Codex CLI installation — Not needed. As explained in my other reply, @openai/codex-sdk already depends on @openai/codex (which ships the CLI binary), so it's installed automatically via npm ci. The .codex directory (line 36-38) is for runtime configuration (e.g., config.toml), not because the CLI needs to be installed globally.

Re: version pinning — This is a valid point for strict reproducibility. However, we intentionally leave it unpinned to pick up the latest compatible CLI version on each build, which is acceptable for this project. The package.json already pins the SDK version (@anthropic-ai/claude-agent-sdk: ^0.2.47), and package-lock.json ensures deterministic local dependency installs. If we encounter version-related issues in CI, we can pin it at that time.

No changes needed here.

Copy link

Choose a reason for hiding this comment

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

@kylewanginchina Thanks for the detailed clarification!

Re: Codex CLI — I understand now. The @openai/codex-sdk packages the CLI binary as a dependency, so the global installation is indeed unnecessary. The .codex directory is purely for runtime configuration. That makes sense.

Re: version pinning — Understood. Your strategy to balance staying current with the latest compatible CLI version while pinning SDK versions in package.json is reasonable for this project's needs.

I have two learnings in my knowledge base that now appear to be outdated based on your explanation:

  1. "For Docker deployments, both Claude Code CLI and Codex CLI must be installed globally in the container" (ID: 3aea425e)
  2. "Codex CLI must be installed globally (npm install -g openai/codex) as it is required by codex-sdk" (ID: e95bd248)

Would you like me to remove these and update my knowledge base to reflect that only Claude Code CLI requires global installation, while Codex CLI is bundled with its SDK?


✏️ Learnings added
Learnt from: kylewanginchina
Repo: kylewanginchina/gitlab-claude-webhook PR: 26
File: Dockerfile:26-27
Timestamp: 2026-03-03T09:37:27.345Z
Learning: The Codex CLI is bundled with `openai/codex-sdk` as a dependency and does not require separate global installation in Docker deployments. It's installed automatically via `npm ci`.

Learnt from: kylewanginchina
Repo: kylewanginchina/gitlab-claude-webhook PR: 26
File: Dockerfile:26-27
Timestamp: 2026-03-03T09:37:27.345Z
Learning: The `.codex` directory in Docker deployments is for runtime configuration (e.g., `config.toml`), not for CLI installation purposes.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: CR
Repo: kylewanginchina/gitlab-claude-webhook PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-21T03:59:56.338Z
Learning: Claude Code CLI must be installed globally (`npm install -g anthropic-ai/claude-code`) as it is required by claude-agent-sdk

Learnt from: CR
Repo: kylewanginchina/gitlab-claude-webhook PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-21T03:59:56.338Z
Learning: For Docker deployments, both Claude Code CLI and Codex CLI must be installed globally in the container

Learnt from: CR
Repo: kylewanginchina/gitlab-claude-webhook PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-21T03:59:56.338Z
Learning: Extract `claude` or `codex` instructions from GitLab webhook events (issue descriptions, comments, merge request descriptions, comments)

Learnt from: CR
Repo: kylewanginchina/gitlab-claude-webhook PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-21T03:59:56.338Z
Learning: Codex CLI must be installed globally (`npm install -g openai/codex`) as it is required by codex-sdk

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.

更新npm依赖包安装

1 participant