refactor: update Dockerfiles for server and UI to streamline dependen… #302
Conversation
…cy installation and build process - Modified Dockerfiles to copy package files for all workspaces, enhancing modularity. - Changed dependency installation to skip scripts, preventing unnecessary execution during builds. - Updated build commands to first build packages in dependency order before building the server and UI, ensuring proper build sequence.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughRemoved per-app Dockerfiles, narrowed .claude ignore, added a consolidated root multi-stage Dockerfile and updated docker-compose to target UI/server stages; changed server GH auth check to call Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Server
participant GH_CLI as "gh CLI"
participant GH_API as "GitHub API"
Client->>Server: GET /setup/gh-status
Server->>GH_CLI: exec `gh api user --jq ".login"`
alt gh api returns login
GH_CLI->>Server: username
Server->>Client: 200 { authenticated: true, user: username }
else gh api fails
Server->>GH_CLI: exec `gh auth status`
GH_CLI->>Server: status output
alt status output contains username
Server->>Client: 200 { authenticated: true, user: username }
else
Server->>Client: 200 { authenticated: false }
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (7)
Comment |
Summary of ChangesHello @Shironex, 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 refactors the Dockerfiles for both the server and UI applications to significantly improve the efficiency and reliability of their build processes. By standardizing how package files are copied, skipping non-essential scripts during dependency installation, and enforcing a proper build order for internal libraries, these changes aim to create a more robust and maintainable Docker build environment, ultimately streamlining the development and deployment workflow. Highlights
🧠 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. Using Gemini Code AssistThe 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
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 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
|
Shironex
added
the
Do Not Merge
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the Dockerfiles for the server and ui applications to correctly handle the npm monorepo structure. The changes streamline dependency installation and the build process by first installing all workspace dependencies and then building the shared libraries before building the applications themselves. This is a solid improvement. My review includes a suggestion to improve the maintainability of the Dockerfiles by reducing code duplication, which will be beneficial as the project grows.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/server/Dockerfile (1)
37-43: Architecture mismatch: hardcodedamd64will fail on ARM64 platforms.The GitHub CLI download is hardcoded to
linux_amd64, butnode:20-alpinesupports multiple architectures. Building or running on ARM64 (e.g., Apple Silicon, AWS Graviton) will fail.🔎 Proposed fix using Docker's TARGETARCH
+# Declare TARGETARCH for multi-arch support +ARG TARGETARCH + # Install git, curl, and GitHub CLI (pinned version for reproducible builds) RUN apk add --no-cache git curl && \ GH_VERSION="2.63.2" && \ - curl -L "https://github.com/cli/cli/releases/download/v${GH_VERSION}/gh_${GH_VERSION}_linux_amd64.tar.gz" -o gh.tar.gz && \ + ARCH="${TARGETARCH:-amd64}" && \ + if [ "$ARCH" = "arm64" ]; then ARCH="arm64"; fi && \ + curl -L "https://github.com/cli/cli/releases/download/v${GH_VERSION}/gh_${GH_VERSION}_linux_${ARCH}.tar.gz" -o gh.tar.gz && \ tar -xzf gh.tar.gz && \ - mv "gh_${GH_VERSION}_linux_amd64/bin/gh" /usr/local/bin/gh && \ - rm -rf gh.tar.gz "gh_${GH_VERSION}_linux_amd64" + mv gh_${GH_VERSION}_linux_${ARCH}/bin/gh /usr/local/bin/gh && \ + rm -rf gh.tar.gz gh_${GH_VERSION}_linux_${ARCH}Note:
TARGETARCHis automatically set by Docker BuildKit when usingdocker buildx buildor whenDOCKER_BUILDKIT=1is set.
🧹 Nitpick comments (1)
apps/ui/Dockerfile (1)
12-21: LGTM - Package manifests properly copied for workspace dependency resolution.The explicit copying of package.json files for all library workspaces is necessary for the global
npm cito resolve dependencies correctly. This approach also optimizes Docker layer caching.Optional enhancement for future maintainability:
If workspaces are added frequently, consider using a script to copy package.json files dynamically, though the current explicit approach provides better visibility and control.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/server/Dockerfileapps/ui/Dockerfile
🔇 Additional comments (7)
apps/server/Dockerfile (4)
12-22: LGTM!Copying all workspace
package.jsonfiles is the correct approach for npm workspaces. This ensures dependency resolution works properly duringnpm ci.
24-25: Effective fix for the husky issue.Using
--ignore-scriptscorrectly skips thepreparelifecycle script that was causing the build failure. The comment explains the rationale well.Note:
--ignore-scriptsalso skipspostinstallscripts for all packages. If any dependency relies onpostinstallfor native compilation beyond node-gyp's automatic binding, verify that the build completes successfully.
27-29: LGTM!Copying the entire
libsdirectory is necessary to support thebuild:packagesstep that follows.
31-32: LGTM!The build sequence correctly builds library packages first, then the server. This ensures dependencies are available in the correct order and compensates for skipping
build:packagesin the prepare script.apps/ui/Dockerfile (3)
27-29: LGTM - Source files properly copied for multi-package build.Copying the entire
libsdirectory is necessary for thebuild:packagescommand to build all workspace dependencies. Since this occurs only in the builder stage and the production stage copies only the final dist artifacts, there's no bloat concern.
31-38: LGTM - Two-phase build ensures correct dependency order.The sequential build approach (
build:packagesfollowed by workspace-specific build) correctly handles the monorepo dependency graph. The environment variables and build arguments are properly configured for the Vite build process.
24-25: Fix correctly prevents husky error in web UI Docker build.The
--ignore-scriptsflag appropriately skips the prepare script (which runs husky and build:packages) since the husky pre-commit hook is not needed during container builds. The explicitnpm run build:packagescall on line 38 ensures workspace packages are still built. This approach is correct for this web-only Docker build, as confirmed byVITE_SKIP_ELECTRON=trueon line 36, which indicates the electron-builder postinstall script is also not required.
- Create unified Dockerfile with multi-stage builds (base, server, ui targets) - Centralize lib package.json COPYs in shared base stage (DRY) - Add Claude CLI installation for Docker authentication support - Remove duplicate apps/server/Dockerfile and apps/ui/Dockerfile - Update docker-compose.yml to use target: parameter - Add docker-compose.override.yml to .gitignore Build commands: docker build --target server -t automaker-server . docker build --target ui -t automaker-ui . docker-compose build && docker-compose up -d 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Dockerfile (1)
21-28: Consider documenting lib path maintenance.Hardcoding lib paths requires manual updates when new workspace packages are added. The comment "centralized - add new libs here" helps, but consider adding a note in the PR description or a README about this requirement.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.gitignoreDockerfileapps/server/Dockerfileapps/ui/Dockerfiledocker-compose.yml
💤 Files with no reviewable changes (2)
- apps/server/Dockerfile
- apps/ui/Dockerfile
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (7)
.gitignore (1)
82-83: LGTM!Ignoring the override file within
.claude/is appropriate for local docker-compose customizations.Dockerfile (4)
41-42: Good fix for the husky issue.Using
--ignore-scriptsto skip husky/prepare scripts, then explicitly rebuildingnode-ptyis the correct approach to fix issue #301 while ensuring native modules work.
123-128: Default VITE_SERVER_URL may not work in Docker Compose.The default
http://localhost:3008works for local development but in Docker Compose, the UI runs in a browser context wherelocalhostrefers to the user's machine, not the server container. This is actually correct for the current setup where both ports are exposed to the host.
138-139: nginx.conf file is present and correctly referenced.The
apps/ui/nginx.conffile exists in the repository, so the COPY command on line 139 will work as expected.
64-65: Confirmed:@anthropic-ai/claude-codeis the correct package name and is required for server functionality.The package exists on npm (version 2.0.76) and is essential to the application—the codebase has extensive Claude CLI integration for usage tracking, authentication, and long-running task handling across multiple services and setup flows.
docker-compose.yml (2)
14-17: LGTM!Using the root Dockerfile with
target: uicorrectly references the multi-stage build.
27-30: LGTM!Using the root Dockerfile with
target: servercorrectly references the multi-stage build.
Use gh api user to verify authentication instead of gh auth status, which can return non-zero even when GH_TOKEN is valid (due to stale config file entries). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/server/src/routes/setup/routes/gh-status.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always import from shared packages (@automaker/*), never from old relative paths
Files:
apps/server/src/routes/setup/routes/gh-status.ts
apps/server/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
createEventEmitter()fromlib/events.tsfor all server operations to emit events that stream to frontend via WebSocket
Files:
apps/server/src/routes/setup/routes/gh-status.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
resolveModelString()from @automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names
Files:
apps/server/src/routes/setup/routes/gh-status.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (1)
apps/server/src/routes/setup/routes/gh-status.ts (1)
100-100: The code already handles version compatibility with a fallback mechanism.The
--jqflag ongh apiis supported and documented in gh CLI, with a proper try-catch fallback togh auth statusif the API call fails. The version is captured (line 77-82) but not validated against a minimum, which is acceptable since the fallback approach ensures the code works across versions. No documentation updates are needed.
- Install bash in Alpine for terminal feature to work - Add dynamic architecture detection for GitHub CLI download (supports x86_64 and aarch64/arm64) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use system-level gitconfig to set safe.directory='*' so it works with mounted volumes and isn't overwritten by user's mounted .gitconfig. Fixes git "dubious ownership" errors when working with projects mounted from the host. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add system-level git config to use `gh auth git-credential` for HTTPS authentication. This allows git push/pull to work automatically using the GH_TOKEN environment variable. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
Dockerfile (1)
110-112: Installwgetexplicitly for the HEALTHCHECK command.The
node:20-alpineimage only includes BusyBox's minimalwgetimplementation, which may behave differently than the full package. For reliable health checks, install wget explicitly.🔎 Proposed fix
Add
wgetto the apk install on line 57:-RUN apk add --no-cache git curl bash && \ +RUN apk add --no-cache git curl bash wget && \Alternatively, use
curlwhich is already installed:HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \ - CMD wget --no-verbose --tries=1 --spider http://localhost:3008/api/health || exit 1 + CMD curl -f http://localhost:3008/api/health || exit 1
🧹 Nitpick comments (2)
Dockerfile (2)
70-71: Consider pinning the Claude CLI version for reproducible builds.The GitHub CLI is pinned to version
2.63.2, but the Claude CLI is unpinned. For consistent, reproducible builds, consider specifying a version.-RUN npm install -g @anthropic-ai/claude-code +RUN npm install -g @anthropic-ai/claude-code@<version>
143-153: Consider adding a health check for the UI container for consistency.The server stage includes a
HEALTHCHECK, but the UI stage does not. For consistency and orchestration reliability (e.g., Kubernetes readiness probes, docker-compose health checks), consider adding one:EXPOSE 80 +HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \ + CMD wget --no-verbose --tries=1 --spider http://localhost:80/ || exit 1 + CMD ["nginx", "-g", "daemon off;"]The
apps/ui/nginx.confis properly configured for SPA routing withtry_files $uri $uri/ /index.html;, ensuring client-side routing works correctly.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Dockerfile
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (4)
Dockerfile (4)
8-31: Well-structured base stage with good layer caching.The approach of copying only
package*.jsonfiles before installing dependencies optimizes Docker layer caching. The build dependencies (python3,make,g++) are correctly included for native module compilation (node-pty).
36-49: Correct fix for the Docker build failure (Issue #301).Using
npm ci --ignore-scriptsto skip lifecycle scripts (husky/prepare) followed by explicitnpm rebuild node-ptyis the right approach. This prevents thesh: husky: not founderror while ensuring native modules are still properly compiled.
95-97: Acceptable security trade-off for container volume mounts.Setting
safe.directory '*'at system level is permissive but appropriate for containers where host volumes may have different ownership. The commit message confirms this addresses "dubious ownership" errors with mounted projects.Ensure this configuration aligns with your security posture—it allows git operations on any directory within the container.
120-138: Good use of build-time configuration for the API endpoint.The
ARG VITE_SERVER_URLwith a sensible default allows overriding the API server URL at build time without modifying the Dockerfile. The--ignore-scriptsflag maintains consistency with the server build approach.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Dockerfile (1)
112-114: Health check will fail: wget is not installed.The HEALTHCHECK command uses
wgetbut it was never installed in this stage. Line 57 only installsgit,curl, andbash. Sincecurlis already available, use it instead for the health check.🔎 Proposed fix using curl
-# Health check -HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \ - CMD wget --no-verbose --tries=1 --spider http://localhost:3008/api/health || exit 1 +# Health check +HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \ + CMD curl -f http://localhost:3008/api/health || exit 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Dockerfile
🔇 Additional comments (4)
Dockerfile (4)
11-31: BASE stage is well-structured and follows Docker best practices.The common setup correctly installs build dependencies for native modules and copies package.json files before source code, enabling effective layer caching. The centralized workspace setup promotes maintainability.
36-49: Correctly addresses the Docker build failure from issue #301.The
--ignore-scriptsflag prevents husky from being invoked during container builds, resolving the "sh: husky: not found" error. The subsequentnpm rebuild node-ptyensures native modules are properly compiled. Build order is correct.
122-140: UI build stage correctly implements the same fix as the server.The
--ignore-scriptsflag prevents husky issues during UI builds. Build args are properly configured to allow runtime customization of the API server URL. Build order is correct.
145-155: UI production stage follows nginx best practices.The nginx-based serving is appropriate for a production SPA. The referenced
apps/ui/nginx.confexists and correctly implements SPA routing with thetry_filesdirective to fallback toindex.htmlfor client-side routing.
- Upgraded base and server images in Dockerfile from Node.js 20 to 22-alpine for better performance and security. - Replaced wget with curl in the health check command for improved reliability. - Enhanced README with detailed Docker deployment instructions, including configuration for API key and Claude CLI authentication, and examples for working with projects and GitHub CLI authentication. This update ensures a more secure and efficient Docker setup for the application.
Shironex
added
Ready-To-Merge
2 participants
Fix Docker compose build process
Resolve
Summary by CodeRabbit
Chores
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.