feat: 添加Linux部署脚本幂等性，更新README.md #566
Conversation
📝 Walkthrough总体概述README.md增加了关于部署脚本仅用于初始部署的信息说明。scripts/deploy.sh新增了现有安装检测函数、全局标志FORCE_REINSTALL和-f/--force命令行选项，支持强制重新安装流程，包括停止和删除现有容器及部署目录的能力。 变更
代码审查工作量评估🎯 2 (Simple) | ⏱️ ~12 分钟 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Summary of ChangesHello @1oca1h0st, 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! 此拉取请求通过为 Highlights
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
|
github-actions
bot
added
size/S
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
|
|
||
| - ⚠️ 请妥善保存脚本输出的**管理员令牌**（Admin Token），这是登录后台的唯一凭证！ | ||
| - ⚠️ Windows 用户：如果未安装 Docker Desktop，脚本会自动打开下载页面 | ||
| - ⚠️ 软件更新：请参考 `Docker Compose 启动章节`，deploy脚本仅用于首次部署，并不用于更新软件 (环境限制，目前仅暂在 Linux 安装脚本确保了部署幂等性，未覆盖 Powershell 脚本) |
There was a problem hiding this comment.
当前的说明文字有些冗长，并且将操作指引和实现细节的备注混合在了一起，可能会让用户困惑。
建议将更新操作的指引和脚本幂等性的状态说明分开，使信息更清晰。同时，更新说明中引用的章节可以更精确地指向包含升级指南的 部署指南 章节，而不是 Docker Compose 启动章节。
| - ⚠️ 软件更新：请参考 `Docker Compose 启动章节`，deploy脚本仅用于首次部署，并不用于更新软件 (环境限制，目前仅暂在 Linux 安装脚本确保了部署幂等性，未覆盖 Powershell 脚本) | |
| - ⚠️ 软件更新：`deploy` 脚本主要用于首次部署。如需更新，请参考 `部署指南` 章节中的升级说明。 (注意：目前仅 Linux 脚本支持幂等性检查，Windows 的 PowerShell 脚本暂不支持。) |
| cd "$DEPLOY_DIR" | ||
| if docker compose version &> /dev/null; then | ||
| docker compose down -v 2>/dev/null || true | ||
| else | ||
| docker-compose down -v 2>/dev/null || true | ||
| fi |
There was a problem hiding this comment.
cd "$DEPLOY_DIR" 会改变当前脚本的全局工作目录，这可能会对后续命令产生意料之外的影响。虽然目前脚本的其他部分似乎能够适应这个变化，但这是一种有风险的实践，降低了代码的可维护性。
建议使用子 shell (subshell) () 来隔离目录切换操作。这样可以确保 docker compose down 命令在正确的目录执行，同时不会影响脚本其余部分的执行环境，从而提高脚本的健壮性。
| cd "$DEPLOY_DIR" | |
| if docker compose version &> /dev/null; then | |
| docker compose down -v 2>/dev/null || true | |
| else | |
| docker-compose down -v 2>/dev/null || true | |
| fi | |
| ( | |
| cd "$DEPLOY_DIR" | |
| if docker compose version &> /dev/null; then | |
| docker compose down -v 2>/dev/null || true | |
| else | |
| docker-compose down -v 2>/dev/null || true | |
| fi | |
| ) |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR. The implementation adds idempotency checks to the deployment script, preventing accidental overwrites of existing installations. The code follows established patterns in the codebase.
PR Size: S
- Lines changed: 98
- Files changed: 2
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean (uses established
|| truepattern for cleanup operations) - Type safety - N/A (shell script)
- Documentation accuracy - Clean
- Test coverage - N/A (deployment script)
- Code clarity - Good
Notes
The implementation correctly:
- Detects existing installations via deployment directory and Docker containers
- Provides clear user guidance when an installation is detected
- Offers a
--forceflag for intentional reinstallation - Updates help text and README documentation accordingly
- Uses proper cleanup patterns with error suppression (
|| true) that are standard in bash cleanup operations
Automated review by Claude AI
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR. The implementation adds idempotency checks to the deployment script, preventing accidental overwrites of existing installations. The code follows established patterns in the codebase.
PR Size: S
- Lines changed: 98
- Files changed: 2
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean (uses established
|| truepattern for cleanup operations) - Type safety - N/A (shell script)
- Documentation accuracy - Clean
- Test coverage - N/A (deployment script)
- Code clarity - Good
Notes
The implementation correctly:
- Detects existing installations via deployment directory and Docker containers
- Provides clear user guidance when an installation is detected
- Offers a
--forceflag for intentional reinstallation - Updates help text and README documentation accordingly
- Uses proper cleanup patterns with error suppression (
|| true) that are standard in bash cleanup operations
Automated review by Claude AI
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @README.md:
- Line 152: Fix the wording in the README sentence that currently reads "目前仅暂在
Linux 安装脚本确保了部署幂等性，未覆盖 Powershell 脚本": remove the extra character "暂" so it
reads "目前仅在 Linux 安装脚本确保了部署幂等性，未覆盖 PowerShell 脚本"; also change "Powershell" to
"PowerShell", replace "deploy脚本" with "deploy 脚本" (add a space), and convert any
parentheses around that clause to full-width Chinese parentheses "（）" for
consistency.
In @scripts/deploy.sh:
- Around line 277-297: When FORCE_REINSTALL is true the script runs "docker
compose down -v" which will delete volumes and can drop DB data; before
stopping/removing containers (the block that checks existing_containers and
DEPLOY_DIR and runs "docker compose down -v" / "docker-compose down -v") add an
explicit confirmation step: if the script is running interactively (tty) prompt
the user to confirm destructive removal of volumes, and if non-interactive
require an explicit opt-in environment variable or flag (e.g.,
FORCE_REINSTALL_CONFIRM=yes or --confirm-delete-volumes) otherwise abort with a
clear log_warning that volumes (e.g., Postgres data) will be deleted; ensure the
check references FORCE_REINSTALL, existing_containers, DEPLOY_DIR and the exact
docker compose commands so the destructive down -v only runs after confirmation.
🧹 Nitpick comments (1)
scripts/deploy.sh (1)
306-308: 日志消息与条件逻辑不符。
else分支仅在installation_found为false时执行，但此时不需要提示"Proceeding with fresh installation"，因为后续逻辑已经会继续执行。当前实现正确但略显冗余。此外，建议将 Line 307 的日志级别从
log_success改为log_info，因为"未发现现有安装"并非一个成功操作。♻️ 建议的改进
else - log_success "No existing installation found. Proceeding with fresh installation..." + log_info "No existing installation found. Proceeding with fresh installation..." fi
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
📒 Files selected for processing (2)
README.mdscripts/deploy.sh
⏰ 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). (3)
- GitHub Check: pr-review
- GitHub Check: pr-description
- GitHub Check: pr-label
🔇 Additional comments (4)
scripts/deploy.sh (4)
50-50: LGTM!全局变量
FORCE_REINSTALL声明清晰，与其他 CLI 参数变量风格一致。
66-66: LGTM!
-f/--force选项的帮助信息和参数解析实现正确，与现有参数处理风格保持一致。Also applies to: 133-136
841-846: LGTM!
check_existing_installation的调用位置正确——在detect_os设置默认部署目录和validate_inputs应用自定义目录覆盖之后执行，确保检查使用的是最终的DEPLOY_DIR值。
242-254: LGTM!容器检测逻辑实现良好：
- 使用
docker ps -a包含已停止的容器- 使用
--filter "name=claude-code-hub-"精确匹配项目容器- 错误输出重定向到
/dev/null避免 Docker 未安装时的错误信息
|
|
||
| - ⚠️ 请妥善保存脚本输出的**管理员令牌**（Admin Token），这是登录后台的唯一凭证！ | ||
| - ⚠️ Windows 用户：如果未安装 Docker Desktop，脚本会自动打开下载页面 | ||
| - ⚠️ 软件更新：请参考 `Docker Compose 启动章节`，deploy脚本仅用于首次部署，并不用于更新软件 (环境限制，目前仅暂在 Linux 安装脚本确保了部署幂等性，未覆盖 Powershell 脚本) |
There was a problem hiding this comment.
文案存在小笔误。
"目前仅暂在 Linux" 中的"暂"字应删除或改为"目前仅在"。
📝 建议的修改
-- ⚠️ 软件更新：请参考 `Docker Compose 启动章节`，deploy脚本仅用于首次部署，并不用于更新软件 (环境限制，目前仅暂在 Linux 安装脚本确保了部署幂等性，未覆盖 Powershell 脚本)
+- ⚠️ 软件更新：请参考 `Docker Compose 启动章节`，deploy 脚本仅用于首次部署，并不用于更新软件（环境限制，目前仅在 Linux 安装脚本确保了部署幂等性，未覆盖 PowerShell 脚本）另外建议：
- "Powershell" → "PowerShell"（官方拼写）
- 括号使用中文全角括号
（）以保持一致性 deploy脚本建议改为deploy 脚本，增加空格提升可读性
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - ⚠️ 软件更新：请参考 `Docker Compose 启动章节`，deploy脚本仅用于首次部署，并不用于更新软件 (环境限制，目前仅暂在 Linux 安装脚本确保了部署幂等性，未覆盖 Powershell 脚本) | |
| - ⚠️ 软件更新：请参考 `Docker Compose 启动章节`，deploy 脚本仅用于首次部署，并不用于更新软件（环境限制，目前仅在 Linux 安装脚本确保了部署幂等性，未覆盖 PowerShell 脚本） |
🤖 Prompt for AI Agents
In @README.md at line 152, Fix the wording in the README sentence that currently
reads "目前仅暂在 Linux 安装脚本确保了部署幂等性，未覆盖 Powershell 脚本": remove the extra character
"暂" so it reads "目前仅在 Linux 安装脚本确保了部署幂等性，未覆盖 PowerShell 脚本"; also change
"Powershell" to "PowerShell", replace "deploy脚本" with "deploy 脚本" (add a space),
and convert any parentheses around that clause to full-width Chinese parentheses
"（）" for consistency.
| # If forcing reinstall, clean up existing installation | ||
| if [[ "$installation_found" == true ]] && [[ "$FORCE_REINSTALL" == true ]]; then | ||
| log_warning "Force reinstall enabled. Removing existing installation..." | ||
|
|
||
| # Stop and remove containers | ||
| if [[ ${#existing_containers[@]} -gt 0 ]]; then | ||
| log_info "Stopping and removing existing containers..." | ||
| if [[ -d "$DEPLOY_DIR" ]] && [[ -f "$DEPLOY_DIR/docker-compose.yaml" ]]; then | ||
| cd "$DEPLOY_DIR" | ||
| if docker compose version &> /dev/null; then | ||
| docker compose down -v 2>/dev/null || true | ||
| else | ||
| docker-compose down -v 2>/dev/null || true | ||
| fi | ||
| fi | ||
|
|
||
| # Force remove any remaining containers | ||
| for container in "${existing_containers[@]}"; do | ||
| docker rm -f "$container" 2>/dev/null || true | ||
| done | ||
| fi |
There was a problem hiding this comment.
数据丢失风险：--force 选项会删除数据库数据，建议添加确认提示。
docker compose down -v 会删除所有卷（包括 PostgreSQL 数据）。在非交互模式下使用 --force 可能导致用户意外丢失生产数据。
建议：在非交互模式下添加额外确认，或至少在帮助信息中明确警告数据会被删除。
🔧 建议的改进方案
# If forcing reinstall, clean up existing installation
if [[ "$installation_found" == true ]] && [[ "$FORCE_REINSTALL" == true ]]; then
+ if [[ "$NON_INTERACTIVE" != true ]]; then
+ echo ""
+ log_warning "This will DELETE all existing data including the database!"
+ read -p "Are you sure you want to continue? [y/N]: " confirm
+ if [[ "$confirm" != "y" ]] && [[ "$confirm" != "Y" ]]; then
+ log_info "Aborted by user."
+ exit 0
+ fi
+ else
+ log_warning "Force reinstall in non-interactive mode - all existing data will be deleted!"
+ fi
log_warning "Force reinstall enabled. Removing existing installation..."🤖 Prompt for AI Agents
In @scripts/deploy.sh around lines 277 - 297, When FORCE_REINSTALL is true the
script runs "docker compose down -v" which will delete volumes and can drop DB
data; before stopping/removing containers (the block that checks
existing_containers and DEPLOY_DIR and runs "docker compose down -v" /
"docker-compose down -v") add an explicit confirmation step: if the script is
running interactively (tty) prompt the user to confirm destructive removal of
volumes, and if non-interactive require an explicit opt-in environment variable
or flag (e.g., FORCE_REINSTALL_CONFIRM=yes or --confirm-delete-volumes)
otherwise abort with a clear log_warning that volumes (e.g., Postgres data) will
be deleted; ensure the check references FORCE_REINSTALL, existing_containers,
DEPLOY_DIR and the exact docker compose commands so the destructive down -v only
runs after confirmation.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if [[ "$installation_found" == true ]] && [[ "$FORCE_REINSTALL" == true ]]; then | ||
| log_warning "Force reinstall enabled. Removing existing installation..." | ||
|
|
||
| # Stop and remove containers | ||
| if [[ ${#existing_containers[@]} -gt 0 ]]; then | ||
| log_info "Stopping and removing existing containers..." | ||
| if [[ -d "$DEPLOY_DIR" ]] && [[ -f "$DEPLOY_DIR/docker-compose.yaml" ]]; then | ||
| cd "$DEPLOY_DIR" | ||
| if docker compose version &> /dev/null; then | ||
| docker compose down -v 2>/dev/null || true | ||
| else | ||
| docker-compose down -v 2>/dev/null || true | ||
| fi | ||
| fi | ||
|
|
||
| # Force remove any remaining containers | ||
| for container in "${existing_containers[@]}"; do | ||
| docker rm -f "$container" 2>/dev/null || true | ||
| done | ||
| fi | ||
|
|
||
| # Remove deployment directory | ||
| if [[ -d "$DEPLOY_DIR" ]]; then | ||
| log_info "Removing deployment directory: $DEPLOY_DIR" | ||
| rm -rf "$DEPLOY_DIR" | ||
| fi |
There was a problem hiding this comment.
The --force reinstall removes the existing deployment directory without preserving credentials. This causes data loss of the admin token and database password from the original .env file. When users run --force, they'll lose access to their existing database and admin panel.
Issue: Lines 300-302 delete $DEPLOY_DIR which contains the .env file with original credentials, but lines 860-862 generate new random credentials. The new credentials won't match the existing database data in Docker volumes (if those volumes survive).
Fix: Before removing the directory, backup existing credentials from the .env file and restore them after cleanup. Extract the admin token and database password values, then modify generate_admin_token() and generate_db_password() functions to skip generation if values are already set from backup.
1 participant
Greptile Overview
Greptile Summary
Adds idempotency check to
deploy.shthat prevents accidental reinstallation by detecting existing installations and requiring a--forceflag for reinstalls. Updates README.md to clarify that the deploy script is for initial deployment only, with software updates requiring manual Docker Compose commands. The force reinstall functionality removes existing containers and deployment directories to enable clean reinstallation.Confidence Score: 2/5
deploy.sh --force, the script deletes the deployment directory (line 302) containing the original.envfile with credentials, then generates completely new random credentials (lines 860-862). This causes users to lose access to their existing database and admin panel. While the idempotency check itself is well-implemented and prevents accidental reinstalls, the force reinstall path has a data loss vulnerability. The README warning is helpful but has minor grammatical issues.Important Files Changed
File Analysis
Sequence Diagram
sequenceDiagram participant User participant Script as deploy.sh participant Check as Installation Check participant Docker participant System as File System User->>Script: "Execute deploy.sh [--force]" Script->>Script: "Parse arguments & detect OS" Script->>Check: "check_existing_installation()" Check->>System: "Check if $DEPLOY_DIR exists" Check->>Docker: "Check for claude-code-hub-* containers" alt Installation exists & no --force flag Check-->>User: "Error: Already installed + instructions" Note over User: Exit with management commands else Installation exists & --force flag Check->>Docker: "docker compose down -v" Check->>Docker: "Remove containers forcefully" Check->>System: "rm -rf $DEPLOY_DIR" Note over Check: WARNING: Credentials lost here! Check->>Script: "Continue with fresh install" else No installation found Check->>Script: "Proceed with installation" end Script->>Script: "Generate new ADMIN_TOKEN" Script->>Script: "Generate new DB_PASSWORD" Script->>System: "Create .env with new credentials" Script->>Docker: "docker compose up -d" Docker-->>User: "Services started"