fix: 解决 3 个 will_fix issues (#24 #22 #56)#62
Open
zhanghuaiwei wants to merge 7 commits into
Open
Conversation
- 新增 pytest.ini 配置 Django 测试环境 - 添加 pytest/pytest-django/pytest-mock 依赖 - 测试环境自动使用内存 SQLite，避免影响开发数据库 Co-Authored-By: Claude <noreply@anthropic.com>
5 个测试覆盖关键容错场景： - 非 JSON payload 解析不崩溃 - 数据库写入异常时视图不崩溃 - GitLab 配置缺失时审查线程优雅失败 - 多种输入场景均返回 200 - 正常流程始终创建 WebhookLog 记录 Co-Authored-By: Claude <noreply@anthropic.com>
LLM 策略模式重构：
- LLMService.review_code() 增加 provider dispatch 分发逻辑
- 提取 _review_with_claude_cli/_review_with_mock/_review_with_api_provider
- unionllm 优先调用，litellm 自动回退
- 统一输出格式 {content, score, issues, metadata}
- LLMConfig.provider 新增 unionllm/litellm 选项
- process_merge_request_review 支持 API provider 路径
Webhook 容错修复：
- gitlab_webhook 所有错误路径返回 200 防止 GitLab 重试
Co-Authored-By: Claude <noreply@anthropic.com>
- push/PR 到 main/master 时自动运行 - 仅 backend/** 变更时触发 - Python 3.11 + pip 缓存 Co-Authored-By: Claude <noreply@anthropic.com>
- 修复 _filter_changes 文件过滤反转逻辑 - LLMConfig 新增 timeout 字段，unionllm/litellm 调用增加超时保护 - 修复 webhook 异常处理中 review.save() 静默吞异常问题 Closes mimo-x#24
- 新增 custom_webhook 通用通知类型，支持模板变量和自定义 HTTP 方法 - 飞书通知增强：HMAC-SHA256 签名校验 + 富文本卡片消息 - 前端新增 custom_webhook 配置表单 Closes mimo-x#22
- 新增 scripts/init-claude.sh 一键初始化脚本（OS 检查/安装/验证/CLAUDE.md/.env） - Dockerfile 构建时自动验证 Claude CLI 安装 Closes mimo-x#56
📝 WalkthroughWalkthroughThis PR extends the code review system with multi-provider LLM support (UnionLLM, LiteLLM), custom webhook notifications, improved error handling in webhook processing, and enhanced Feishu interactive cards. Testing infrastructure with pytest and GitHub Actions CI is established, supporting 1,100+ lines of changes across backend, frontend, and configuration. ChangesMulti-Provider LLM Review with Custom Webhook Notifications
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes The PR spans heterogeneous changes across backend services (multi-provider LLM dispatch refactor, custom webhook channels, Feishu signature/card support, webhook error handling), frontend UI, and testing infrastructure. Dense logic in LLMService dispatcher and notification handling, provider branching in webhook views, and template substitution in multiple handlers demand separate reasoning for each functional area. Large supporting test suite and configuration changes add breadth. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (9)
frontend/src/views/Config.vue (1)
557-560: ⚡ Quick winConsider adding
.numbermodifier for consistency.Line 228 uses
v-model.numberfor the Claude CLI timeout input, but line 559 forhttp_timeoutuses plainv-model. While browsers withtype="number"inputs often handle this, using.numberensures consistent type coercion.♻️ Suggested fix
- <input v-model="channelForm.http_timeout" type="number" class="input-field" min="1" max="300" placeholder="30" /> + <input v-model.number="channelForm.http_timeout" type="number" class="input-field" min="1" max="300" placeholder="30" />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/views/Config.vue` around lines 557 - 560, The http timeout input in Config.vue is using plain v-model which can leave channelForm.http_timeout as a string; update the input binding for channelForm.http_timeout to use the numeric modifier (v-model.number) so the value is coerced to a Number (match the existing pattern used for the Claude CLI timeout), i.e. change the input with v-model="channelForm.http_timeout" (class "input-field") to use v-model.number on the same property.backend/apps/webhook/tests/test_webhook_views.py (3)
136-161: 💤 Low valueClarify test intent: log creation vs. error handling.
This test verifies that
WebhookLogis created, but the docstring says "无论处理成功或失败" (whether processing succeeds or fails). The test only exercises the success path with a valid payload. Consider either:
- Updating the docstring to match the actual test (success path only)
- Adding a second assertion that tests log creation during failure scenarios
📝 Example: Add failure scenario assertion
log_entry = WebhookLog.objects.latest('created_at') assert log_entry.event_type == 'merge_request' assert log_entry.project_id == 1 + + # Verify log creation also happens on failure path + with patch('apps.webhook.views.handle_webhook_event', side_effect=Exception("test error")): + response = api_client.post( + '/api/webhook/gitlab/', + data=webhook_payload, + format='json', + ) + assert response.status_code == 200 + assert WebhookLog.objects.count() == initial_count + 2🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/webhook/tests/test_webhook_views.py` around lines 136 - 161, The test test_webhook_log_created_on_all_paths currently only exercises the success path but claims to verify "whether processing succeeds or fails"; either update the docstring to reflect it's testing the success path only, or extend the test to also exercise a failure path: after the successful POST using api_client.post('/api/webhook/gitlab/', data=webhook_payload) add a second POST with a malformed/invalid payload (or a payload that triggers handler error) and assert response.status_code indicates failure and that WebhookLog.objects.count() increased again and the latest WebhookLog (WebhooksLog.objects.latest('created_at')) records the failed event (checking event_type/project_id or an error flag), ensuring create_initial_webhook_log behavior is validated in both cases.
9-9: 💤 Low valueRemove unused import.
reverseis imported but never used in the tests. All URL references are hardcoded strings.♻️ Remove unused import
-from django.urls import reverse from django.db.utils import OperationalError🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/webhook/tests/test_webhook_views.py` at line 9, The import of reverse in backend/apps/webhook/tests/test_webhook_views.py is unused; remove the unused symbol by deleting the line "from django.urls import reverse" (or removing reverse from that import statement if other names are added later) so the test module no longer imports reverse unnecessarily and lint/test-suite warnings go away.
24-28: Replace hardcoded GitLab webhook URL paths withreverse()Six tests hardcode
'/api/webhook/gitlab/'; if URL routing changes, they’ll break. Usereverse('webhook:gitlab_webhook')instead (URL isapi/webhook/+gitlab/withapp_name='webhook'andname='gitlab_webhook').File:
backend/apps/webhook/tests/test_webhook_views.py
Lines: 24-28response = api_client.post( '/api/webhook/gitlab/', data=b'\xff\xfe\x00\x01invalid-bytes', content_type='application/octet-stream', )♻️ Example fix
- response = api_client.post( - '/api/webhook/gitlab/', + response = api_client.post( + reverse('webhook:gitlab_webhook'), data=b'\xff\xfe\x00\x01invalid-bytes', content_type='application/octet-stream', )Also update the remaining occurrences at: 46-50, 112-116, 121-126, 129-133, 149-153.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/webhook/tests/test_webhook_views.py` around lines 24 - 28, Tests hardcode the GitLab webhook path '/api/webhook/gitlab/' which will break if routing changes; replace each hardcoded URL used in api_client.post calls with Django's reverse('webhook:gitlab_webhook') to resolve the route dynamically. Locate the api_client.post invocations in test_webhook_views.py (e.g., the call around the data=b'\xff\xfe\x00\x01invalid-bytes' and the other occurrences at the indicated ranges) and pass reverse('webhook:gitlab_webhook') as the first argument instead of the literal string; ensure to import django.urls.reverse at the top of the test module if not already imported..github/workflows/pytest.yml (1)
40-42: 💤 Low valueConsider adding test coverage reporting.
The pytest configuration lacks coverage reporting. Adding
pytest-covand--covflags would help track test coverage over time.📊 Optional enhancement to add coverage
- name: Run pytest run: | - python -m pytest --tb=short -v + python -m pytest --tb=short -v --cov=apps --cov-report=term-missingAlso add to
requirements.txt:pytest-cov==5.0.0🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/pytest.yml around lines 40 - 42, Update the "Run pytest" step so pytest runs with coverage: modify the command in the Run pytest step (the line currently running "python -m pytest --tb=short -v") to include pytest-cov flags (e.g. add "--cov=./" and a report flag like "--cov-report=xml" or "--cov-report=term-missing") and ensure pytest-cov is added to dependencies (add "pytest-cov==5.0.0" to requirements.txt) so CI produces coverage outputs for tracking.backend/core/settings.py (1)
76-76: ⚡ Quick winMove the
import sysstatement to the top of the file.Python imports should be grouped at the module top (after the docstring) rather than inline. The
sysimport on line 76 should be moved to line 6, alongside the existingosandPathimports.📦 Proposed fix to relocate import
Move line 76 to the import block:
import os +import sys from pathlib import PathThen update line 76-77:
# 测试环境使用内存 SQLite，避免影响开发数据库 -import sys if 'test' in sys.argv or 'pytest' in sys.modules:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/core/settings.py` at line 76, Move the inline "import sys" to the module import block at the top of the file (alongside the existing "import os" and "from pathlib import Path") so all imports are grouped after the module docstring; remove the now-redundant inline "import sys" statement and ensure only the top-level import remains.backend/apps/webhook/views.py (1)
750-750: 💤 Low value
prompt_lengthis always 0, reducing observability.The
prompt_lengthmetric is hardcoded to0because the prompt is built insidellm_service.review_code(). Consider having the LLM service return prompt length in its metadata for more accurate logging.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/webhook/views.py` at line 750, The prompt_length metric is hardcoded to 0 because the prompt is constructed inside llm_service.review_code(); update the contract of llm_service.review_code to return prompt metadata (e.g., include prompt_length or prompt_text in its response/metadata), then use that returned value when calling the metrics recorder (replace prompt_length=0 with prompt_length=metadata.get("prompt_length") or len(metadata["prompt_text"])) so the logged prompt_length reflects the actual prompt built by review_code.backend/apps/llm/services.py (1)
588-628: 💤 Low valueConsider removing unused methods.
_review_with_openaiand_build_promptappear to be dead code after the refactor to the new multi-provider dispatcher (_review_with_api_provider). The new flow uses_try_unionllmand_try_litellminstead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/llm/services.py` around lines 588 - 628, These two methods (_review_with_openai and _build_prompt) are now unused after the refactor to the multi-provider dispatcher (_review_with_api_provider) and should be removed; delete both methods from the class (and any local imports like "from openai import OpenAI" inside _review_with_openai), remove or update any references to them elsewhere (search for _review_with_openai and _build_prompt), and run tests/static analysis to ensure no remaining usages or import errors after removal.backend/apps/llm/migrations/0009_add_llmconfig_timeout.py (1)
15-15: 💤 Low valueConsider using halfwidth punctuation for consistency in technical strings.
The help text uses Chinese fullwidth punctuation
（），which is common in Chinese prose but can cause visual inconsistencies in technical contexts. Consider using halfwidth(),for better consistency with code and technical documentation.This is a minor style concern and doesn't affect functionality.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/llm/migrations/0009_add_llmconfig_timeout.py` at line 15, Update the help_text string on the models.IntegerField declaration (the field=models.IntegerField(default=600, help_text=...)) to replace the Chinese fullwidth punctuation characters （ and ） and the fullwidth comma with standard halfwidth punctuation (use parentheses () and comma ,) so the help text reads with ASCII punctuation for consistency in technical strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pytest.yml:
- Around line 21-34: Replace mutable action tags with immutable commit SHAs and
disable credential persistence for checkout: update the uses entries for
actions/checkout and actions/setup-python to their corresponding commit SHA pins
(instead of `@v4/`@v5) and add with: persist-credentials: false to the
actions/checkout invocation; also consider pinning actions/cache to a SHA for
full hardening and keep the existing setup-python inputs (python-version)
intact.
In `@backend/apps/llm/services.py`:
- Around line 101-104: The litellm branch only sets LITELLM_API_KEY but misses
applying a custom API base; update the provider handling (in the method where
self.provider is checked, e.g., the litellm branch in the service method that
references self.api_key, self.api_base, self.request_id, and logger) to also set
os.environ['LITELLM_API_BASE'] = self.api_base when self.api_base is present and
add a corresponding logger.info line (similar to UNIONLLM_API_BASE handling) so
custom API base URLs are applied for the litellm provider.
In `@backend/apps/llm/views.py`:
- Around line 504-514: Add a shared validation utility (e.g.,
validate_custom_webhook(url, headers)) and call it from both
_test_custom_webhook (views.py) and _send_to_custom_webhook
(notification_dispatcher.py); the validator must check the URL scheme is
http/https, parse the host and reject localhost/127.0.0.1/::1, RFC1918 private
ranges, and known cloud metadata endpoints (169.254.169.254 and similar), and
optionally perform DNS resolution to ensure the resolved IP is not private; also
validate headers by forbidding hop-by-hop and sensitive header names (Host,
Content-Length, Transfer-Encoding, Connection, Proxy-Authorization,
Proxy-Authenticate), ensure header names match token syntax and header values
contain no CR/LF or control chars and enforce a reasonable size limit; make the
functions use this validator and return a 400 response or raise a
ValidationError when validation fails so both endpoints share identical
defense-in-depth logic.
- Around line 513-514: The code reads headers = config_dict.get('headers', {})
and passes them straight into outgoing HTTP requests; add validation before use:
ensure header keys match a safe pattern (e.g., only letters, digits, hyphens),
header values are strings, trim whitespace, reject or drop any header with CR/LF
or other control characters, and enforce reasonable length limits; perform this
sanitization in the same view (before the request is sent) and log or raise a
clear error for invalid headers so they are not forwarded to the HTTP client.
- Around line 504-510: The webhook URL extracted into variable url from
config_dict must be validated to prevent SSRF: parse url (using urlparse) and
verify scheme is http or https; reject empty or non-http(s) schemes; resolve the
hostname to one or more IPs (socket.getaddrinfo) and use ipaddress to reject
loopback, link-local, private RFC1918 ranges, and known metadata IPs like
169.254.169.254; if any resolved IP is disallowed, return the same Response
pattern (success False, message) with status.HTTP_400_BAD_REQUEST instead of
proceeding; reference the url/config_dict extraction and the existing Response
block in backend/apps/llm/views.py to implement these checks.
In `@backend/apps/response/notification_dispatcher.py`:
- Around line 696-706: The duplicate apply_template implementation should be
extracted into a shared utility: create a new function
apply_webhook_template(obj, template_vars) in a new template_utils module
containing the recursive string/dict/list replacement logic, then replace the
local apply_template definitions with an import and call to
apply_webhook_template in both places where apply_template is duplicated
(replace the local function bodies and usages), ensuring the new utility accepts
the same inputs and preserves behavior for str, dict, list, and other types.
In `@docker/backend/Dockerfile`:
- Around line 15-16: The Dockerfile RUN step silently succeeds when the Claude
CLI install/version check fails because of the trailing "|| echo ..." — change
the RUN so that the install command and the version check must both succeed
(e.g., remove the "|| echo ..." and chain with && so "claude --version" returns
a non‑zero exit to fail the build) and ensure the shell in that RUN uses a
failing-on-error behavior; additionally, inspect the init script copied later
(the script referenced by the COPY that sets "project root") and confirm its
project-root resolution logic will point to the intended runtime directory
(adjust path resolution in that script if it relies on build-time paths).
- Around line 19-20: The init-claude script deriving PROJECT_ROOT from its
install location causes it to treat /usr/local as the repo root when copied to
/usr/local/bin; update the Dockerfile and/or the script so PROJECT_ROOT
correctly points to the repository root: either (A) copy scripts/init-claude.sh
into a path under /app (e.g., /app/scripts/) so PROJECT_ROOT resolves to /app,
and also COPY the repo-level files .env and CLAUDE.md into the image or the /app
location, or (B) change init-claude.sh’s PROJECT_ROOT logic to prefer an
explicit override (ENV var or CLI flag like INIT_CLAUDE_ROOT) or to detect a
mounted repo root (search upward for .env/CLAUDE.md instead of using
$SCRIPT_DIR/..). Update references to PROJECT_ROOT in init-claude.sh accordingly
and ensure the Dockerfile provides the chosen override or copies the needed repo
files.
---
Nitpick comments:
In @.github/workflows/pytest.yml:
- Around line 40-42: Update the "Run pytest" step so pytest runs with coverage:
modify the command in the Run pytest step (the line currently running "python -m
pytest --tb=short -v") to include pytest-cov flags (e.g. add "--cov=./" and a
report flag like "--cov-report=xml" or "--cov-report=term-missing") and ensure
pytest-cov is added to dependencies (add "pytest-cov==5.0.0" to
requirements.txt) so CI produces coverage outputs for tracking.
In `@backend/apps/llm/migrations/0009_add_llmconfig_timeout.py`:
- Line 15: Update the help_text string on the models.IntegerField declaration
(the field=models.IntegerField(default=600, help_text=...)) to replace the
Chinese fullwidth punctuation characters （ and ） and the fullwidth comma with
standard halfwidth punctuation (use parentheses () and comma ,) so the help text
reads with ASCII punctuation for consistency in technical strings.
In `@backend/apps/llm/services.py`:
- Around line 588-628: These two methods (_review_with_openai and _build_prompt)
are now unused after the refactor to the multi-provider dispatcher
(_review_with_api_provider) and should be removed; delete both methods from the
class (and any local imports like "from openai import OpenAI" inside
_review_with_openai), remove or update any references to them elsewhere (search
for _review_with_openai and _build_prompt), and run tests/static analysis to
ensure no remaining usages or import errors after removal.
In `@backend/apps/webhook/tests/test_webhook_views.py`:
- Around line 136-161: The test test_webhook_log_created_on_all_paths currently
only exercises the success path but claims to verify "whether processing
succeeds or fails"; either update the docstring to reflect it's testing the
success path only, or extend the test to also exercise a failure path: after the
successful POST using api_client.post('/api/webhook/gitlab/',
data=webhook_payload) add a second POST with a malformed/invalid payload (or a
payload that triggers handler error) and assert response.status_code indicates
failure and that WebhookLog.objects.count() increased again and the latest
WebhookLog (WebhooksLog.objects.latest('created_at')) records the failed event
(checking event_type/project_id or an error flag), ensuring
create_initial_webhook_log behavior is validated in both cases.
- Line 9: The import of reverse in
backend/apps/webhook/tests/test_webhook_views.py is unused; remove the unused
symbol by deleting the line "from django.urls import reverse" (or removing
reverse from that import statement if other names are added later) so the test
module no longer imports reverse unnecessarily and lint/test-suite warnings go
away.
- Around line 24-28: Tests hardcode the GitLab webhook path
'/api/webhook/gitlab/' which will break if routing changes; replace each
hardcoded URL used in api_client.post calls with Django's
reverse('webhook:gitlab_webhook') to resolve the route dynamically. Locate the
api_client.post invocations in test_webhook_views.py (e.g., the call around the
data=b'\xff\xfe\x00\x01invalid-bytes' and the other occurrences at the indicated
ranges) and pass reverse('webhook:gitlab_webhook') as the first argument instead
of the literal string; ensure to import django.urls.reverse at the top of the
test module if not already imported.
In `@backend/apps/webhook/views.py`:
- Line 750: The prompt_length metric is hardcoded to 0 because the prompt is
constructed inside llm_service.review_code(); update the contract of
llm_service.review_code to return prompt metadata (e.g., include prompt_length
or prompt_text in its response/metadata), then use that returned value when
calling the metrics recorder (replace prompt_length=0 with
prompt_length=metadata.get("prompt_length") or len(metadata["prompt_text"])) so
the logged prompt_length reflects the actual prompt built by review_code.
In `@backend/core/settings.py`:
- Line 76: Move the inline "import sys" to the module import block at the top of
the file (alongside the existing "import os" and "from pathlib import Path") so
all imports are grouped after the module docstring; remove the now-redundant
inline "import sys" statement and ensure only the top-level import remains.
In `@frontend/src/views/Config.vue`:
- Around line 557-560: The http timeout input in Config.vue is using plain
v-model which can leave channelForm.http_timeout as a string; update the input
binding for channelForm.http_timeout to use the numeric modifier
(v-model.number) so the value is coerced to a Number (match the existing pattern
used for the Claude CLI timeout), i.e. change the input with
v-model="channelForm.http_timeout" (class "input-field") to use v-model.number
on the same property.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0ba81996-43e6-4254-9214-390d2ba510c3
📒 Files selected for processing (21)
.github/workflows/pytest.ymlbackend/apps/llm/migrations/0008_add_unionllm_litellm_providers.pybackend/apps/llm/migrations/0009_add_llmconfig_timeout.pybackend/apps/llm/migrations/0010_add_custom_webhook.pybackend/apps/llm/models.pybackend/apps/llm/serializers.pybackend/apps/llm/services.pybackend/apps/llm/views.pybackend/apps/response/notification_dispatcher.pybackend/apps/response/services.pybackend/apps/review/services.pybackend/apps/webhook/tests/__init__.pybackend/apps/webhook/tests/conftest.pybackend/apps/webhook/tests/test_webhook_views.pybackend/apps/webhook/views.pybackend/core/settings.pybackend/pytest.inibackend/requirements.txtdocker/backend/Dockerfilefrontend/src/views/Config.vuescripts/init-claude.sh
Comment on lines
+21
to
+34
| - uses: actions/checkout@v4 | ||
|
|
||
| - uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: '3.11' | ||
|
|
||
| - name: Cache pip packages | ||
| uses: actions/cache@v4 | ||
| with: | ||
| path: ~/.cache/pip | ||
| key: ${{ runner.os }}-pip-${{ hashFiles('backend/requirements.txt') }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-pip- | ||
|
|
There was a problem hiding this comment.
Pin GitHub Actions to commit SHAs and disable credential persistence.
The workflow uses mutable version tags (@v4, @v5) instead of immutable commit SHAs, creating supply-chain risk. Additionally, actions/checkout should set persist-credentials: false to prevent credentials from being exposed via workflow artifacts.
🔒 Recommended fix to pin actions and disable credentials
- - uses: actions/checkout@v4
+ - uses: actions/checkout@v4
+ with:
+ persist-credentials: false
- - uses: actions/setup-python@v5
+ - uses: actions/setup-python@v5
with:
python-version: '3.11'
- name: Cache pip packages
- uses: actions/cache@v4
+ uses: actions/cache@v4
with:For full supply-chain hardening, also consider pinning to commit SHAs:
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
with:
persist-credentials: false
- uses: actions/setup-python@82c7e631bb3cdc910f68e0081d67478d79c6982d # v5.1.0🧰 Tools
🪛 zizmor (1.25.2)
[warning] 21-21: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 21-21: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 23-23: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 28-28: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/pytest.yml around lines 21 - 34, Replace mutable action
tags with immutable commit SHAs and disable credential persistence for checkout:
update the uses entries for actions/checkout and actions/setup-python to their
corresponding commit SHA pins (instead of `@v4/`@v5) and add with:
persist-credentials: false to the actions/checkout invocation; also consider
pinning actions/cache to a SHA for full hardening and keep the existing
setup-python inputs (python-version) intact.
Source: Linters/SAST tools
Comment on lines
+101
to
+104
| elif self.provider.lower() == 'litellm': | ||
| if self.api_key: | ||
| os.environ['LITELLM_API_KEY'] = self.api_key | ||
| logger.info(f"[{self.request_id}] 设置 LITELLM_API_KEY 环境变量") |
There was a problem hiding this comment.
Missing api_base configuration for litellm provider.
The unionllm provider sets both UNIONLLM_API_KEY and UNIONLLM_API_BASE, but litellm only sets LITELLM_API_KEY. If users configure a custom API base URL for litellm, it won't be applied.
🔧 Proposed fix
elif self.provider.lower() == 'litellm':
if self.api_key:
os.environ['LITELLM_API_KEY'] = self.api_key
logger.info(f"[{self.request_id}] 设置 LITELLM_API_KEY 环境变量")
+ if self.api_base:
+ os.environ['LITELLM_API_BASE'] = self.api_base
+ logger.info(f"[{self.request_id}] 设置 LITELLM_API_BASE 环境变量")📝 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.
Suggested change
| elif self.provider.lower() == 'litellm': | |
| if self.api_key: | |
| os.environ['LITELLM_API_KEY'] = self.api_key | |
| logger.info(f"[{self.request_id}] 设置 LITELLM_API_KEY 环境变量") | |
| elif self.provider.lower() == 'litellm': | |
| if self.api_key: | |
| os.environ['LITELLM_API_KEY'] = self.api_key | |
| logger.info(f"[{self.request_id}] 设置 LITELLM_API_KEY 环境变量") | |
| if self.api_base: | |
| os.environ['LITELLM_API_BASE'] = self.api_base | |
| logger.info(f"[{self.request_id}] 设置 LITELLM_API_BASE 环境变量") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/apps/llm/services.py` around lines 101 - 104, The litellm branch only
sets LITELLM_API_KEY but misses applying a custom API base; update the provider
handling (in the method where self.provider is checked, e.g., the litellm branch
in the service method that references self.api_key, self.api_base,
self.request_id, and logger) to also set os.environ['LITELLM_API_BASE'] =
self.api_base when self.api_base is present and add a corresponding logger.info
line (similar to UNIONLLM_API_BASE handling) so custom API base URLs are applied
for the litellm provider.
Comment on lines
+504
to
+510
| url = config_dict.get('url') or config_dict.get('webhook_url') or config_dict.get('webhook') | ||
|
|
||
| if not url: | ||
| return Response({ | ||
| 'success': False, | ||
| 'message': '自定义Webhook URL未配置，请在config_data中设置 url 字段' | ||
| }, status=status.HTTP_400_BAD_REQUEST) |
There was a problem hiding this comment.
Add URL validation to prevent SSRF attacks.
The custom webhook URL is extracted from admin configuration without validation. While this is admin-configured data (not direct user input), a compromised admin account could configure webhooks targeting internal services (localhost, 127.0.0.1, internal IP ranges, cloud metadata endpoints).
🔒 Recommended: Add URL validation before making the request
+from urllib.parse import urlparse
+from ipaddress import ip_address, ip_network
+
+def _is_safe_url(url: str) -> tuple[bool, str]:
+ """Validate URL is not targeting internal services"""
+ try:
+ parsed = urlparse(url)
+ host = parsed.hostname
+
+ if not host:
+ return False, "Invalid URL: no hostname"
+
+ # Block localhost aliases
+ if host.lower() in ['localhost', 'localhost.localdomain']:
+ return False, "Internal URL blocked: localhost"
+
+ # Block private IP ranges
+ try:
+ addr = ip_address(host)
+ private_ranges = [
+ ip_network('127.0.0.0/8'),
+ ip_network('10.0.0.0/8'),
+ ip_network('172.16.0.0/12'),
+ ip_network('192.168.0.0/16'),
+ ip_network('169.254.0.0/16'), # link-local
+ ip_network('::1/128'), # IPv6 localhost
+ ip_network('fc00::/7'), # IPv6 private
+ ]
+ for private_range in private_ranges:
+ if addr in private_range:
+ return False, f"Internal IP blocked: {host}"
+ except ValueError:
+ # Not an IP address, hostname is OK
+ pass
+
+ return True, ""
+ except Exception as e:
+ return False, f"URL validation error: {e}"
def _test_custom_webhook(self, channel, test_id, timestamp):
...
url = config_dict.get('url') or config_dict.get('webhook_url') or config_dict.get('webhook')
if not url:
return Response({
'success': False,
'message': '自定义Webhook URL未配置，请在config_data中设置 url 字段'
}, status=status.HTTP_400_BAD_REQUEST)
+
+ # Validate URL safety
+ is_safe, error_msg = _is_safe_url(url)
+ if not is_safe:
+ return Response({
+ 'success': False,
+ 'message': f'URL validation failed: {error_msg}'
+ }, status=status.HTTP_400_BAD_REQUEST)🧰 Tools
🪛 Ruff (0.15.15)
[warning] 509-509: String contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/apps/llm/views.py` around lines 504 - 510, The webhook URL extracted
into variable url from config_dict must be validated to prevent SSRF: parse url
(using urlparse) and verify scheme is http or https; reject empty or non-http(s)
schemes; resolve the hostname to one or more IPs (socket.getaddrinfo) and use
ipaddress to reject loopback, link-local, private RFC1918 ranges, and known
metadata IPs like 169.254.169.254; if any resolved IP is disallowed, return the
same Response pattern (success False, message) with status.HTTP_400_BAD_REQUEST
instead of proceeding; reference the url/config_dict extraction and the existing
Response block in backend/apps/llm/views.py to implement these checks.
Comment on lines
+504
to
+514
| url = config_dict.get('url') or config_dict.get('webhook_url') or config_dict.get('webhook') | ||
|
|
||
| if not url: | ||
| return Response({ | ||
| 'success': False, | ||
| 'message': '自定义Webhook URL未配置，请在config_data中设置 url 字段' | ||
| }, status=status.HTTP_400_BAD_REQUEST) | ||
|
|
||
| method = config_dict.get('method', 'POST').upper() | ||
| headers = config_dict.get('headers', {}) | ||
| payload_template = config_dict.get('payload_template', {}) |
There was a problem hiding this comment.
Add URL and header validation to custom webhook feature in both views.py and notification_dispatcher.py.
The custom webhook feature processes admin-configured URLs and headers without validation in both the test endpoint (backend/apps/llm/views.py _test_custom_webhook) and the notification dispatcher (backend/apps/response/notification_dispatcher.py _send_to_custom_webhook).
This creates two security posture gaps:
- SSRF risk: URLs could target internal services (localhost, 127.0.0.1, private IP ranges, cloud metadata endpoints)
- Header injection risk: Headers could be crafted to inject malicious values
While the threat model is compromised admin accounts (not external users), adding validation improves defense-in-depth. The same validation logic should be applied in both locations, or ideally extracted to a shared validation utility.
🧰 Tools
🪛 Ruff (0.15.15)
[warning] 509-509: String contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/apps/llm/views.py` around lines 504 - 514, Add a shared validation
utility (e.g., validate_custom_webhook(url, headers)) and call it from both
_test_custom_webhook (views.py) and _send_to_custom_webhook
(notification_dispatcher.py); the validator must check the URL scheme is
http/https, parse the host and reject localhost/127.0.0.1/::1, RFC1918 private
ranges, and known cloud metadata endpoints (169.254.169.254 and similar), and
optionally perform DNS resolution to ensure the resolved IP is not private; also
validate headers by forbidding hop-by-hop and sensitive header names (Host,
Content-Length, Transfer-Encoding, Connection, Proxy-Authorization,
Proxy-Authenticate), ensure header names match token syntax and header values
contain no CR/LF or control chars and enforce a reasonable size limit; make the
functions use this validator and return a 400 response or raise a
ValidationError when validation fails so both endpoints share identical
defense-in-depth logic.
Comment on lines
+513
to
+514
| headers = config_dict.get('headers', {}) | ||
| payload_template = config_dict.get('payload_template', {}) |
There was a problem hiding this comment.
Validate custom headers to prevent header injection.
Headers from config_dict are passed directly to the HTTP request without validation. While this is admin-configured, consider validating header names and values to prevent header injection attacks or misconfiguration.
🔒 Recommended: Add basic header validation
+def _validate_headers(headers: dict) -> tuple[bool, str]:
+ """Validate custom headers"""
+ if not isinstance(headers, dict):
+ return False, "Headers must be a dictionary"
+
+ # Block sensitive headers that could be misused
+ blocked_headers = ['host', 'connection', 'transfer-encoding']
+
+ for key, value in headers.items():
+ if not isinstance(key, str) or not isinstance(value, str):
+ return False, f"Header key and value must be strings: {key}"
+
+ if key.lower() in blocked_headers:
+ return False, f"Header '{key}' is not allowed"
+
+ # Check for newlines (header injection)
+ if '\n' in key or '\r' in key or '\n' in value or '\r' in value:
+ return False, f"Header contains invalid characters: {key}"
+
+ return True, ""
method = config_dict.get('method', 'POST').upper()
headers = config_dict.get('headers', {})
+
+# Validate headers
+is_valid, error_msg = _validate_headers(headers)
+if not is_valid:
+ return Response({
+ 'success': False,
+ 'message': f'Header validation failed: {error_msg}'
+ }, status=status.HTTP_400_BAD_REQUEST)
+
payload_template = config_dict.get('payload_template', {})📝 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.
Suggested change
| headers = config_dict.get('headers', {}) | |
| payload_template = config_dict.get('payload_template', {}) | |
| def _validate_headers(headers: dict) -> tuple[bool, str]: | |
| """Validate custom headers""" | |
| if not isinstance(headers, dict): | |
| return False, "Headers must be a dictionary" | |
| # Block sensitive headers that could be misused | |
| blocked_headers = ['host', 'connection', 'transfer-encoding'] | |
| for key, value in headers.items(): | |
| if not isinstance(key, str) or not isinstance(value, str): | |
| return False, f"Header key and value must be strings: {key}" | |
| if key.lower() in blocked_headers: | |
| return False, f"Header '{key}' is not allowed" | |
| # Check for newlines (header injection) | |
| if '\n' in key or '\r' in key or '\n' in value or '\r' in value: | |
| return False, f"Header contains invalid characters: {key}" | |
| return True, "" | |
| method = config_dict.get('method', 'POST').upper() | |
| headers = config_dict.get('headers', {}) | |
| # Validate headers | |
| is_valid, error_msg = _validate_headers(headers) | |
| if not is_valid: | |
| return Response({ | |
| 'success': False, | |
| 'message': f'Header validation failed: {error_msg}' | |
| }, status=status.HTTP_400_BAD_REQUEST) | |
| payload_template = config_dict.get('payload_template', {}) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/apps/llm/views.py` around lines 513 - 514, The code reads headers =
config_dict.get('headers', {}) and passes them straight into outgoing HTTP
requests; add validation before use: ensure header keys match a safe pattern
(e.g., only letters, digits, hyphens), header values are strings, trim
whitespace, reject or drop any header with CR/LF or other control characters,
and enforce reasonable length limits; perform this sanitization in the same view
(before the request is sent) and log or raise a clear error for invalid headers
so they are not forwarded to the HTTP client.
Comment on lines
+696
to
+706
| def apply_template(obj): | ||
| if isinstance(obj, str): | ||
| result = obj | ||
| for var, value in template_vars.items(): | ||
| result = result.replace(var, value) | ||
| return result | ||
| elif isinstance(obj, dict): | ||
| return {apply_template(k): apply_template(v) for k, v in obj.items()} | ||
| elif isinstance(obj, list): | ||
| return [apply_template(item) for item in obj] | ||
| return obj |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Extract duplicated template replacement logic into a shared utility.
The apply_template function at lines 696-706 is identical to the one in backend/apps/llm/views.py lines 530-540. This template replacement logic is used in both the test endpoint and the actual notification dispatch.
♻️ Recommended: Extract to shared utility module
Create backend/apps/response/template_utils.py:
"""Template variable replacement utilities for custom webhooks"""
from typing import Any, Dict
def apply_webhook_template(obj: Any, template_vars: Dict[str, str]) -> Any:
"""
Recursively replace template variables in strings, dicts, and lists.
Args:
obj: The object to process (str, dict, list, or other)
template_vars: Dictionary mapping template variables to their values
Returns:
Processed object with template variables replaced
"""
if isinstance(obj, str):
result = obj
for var, value in template_vars.items():
result = result.replace(var, value)
return result
elif isinstance(obj, dict):
return {apply_webhook_template(k, template_vars): apply_webhook_template(v, template_vars)
for k, v in obj.items()}
elif isinstance(obj, list):
return [apply_webhook_template(item, template_vars) for item in obj]
return objThen update both files:
In backend/apps/llm/views.py:
+from apps.response.template_utils import apply_webhook_template
def _test_custom_webhook(self, channel, test_id, timestamp):
...
- def apply_template(obj):
- if isinstance(obj, str):
- result = obj
- for var, value in template_vars.items():
- result = result.replace(var, value)
- return result
- elif isinstance(obj, dict):
- return {apply_template(k): apply_template(v) for k, v in obj.items()}
- elif isinstance(obj, list):
- return [apply_template(item) for item in obj]
- return obj
-
- payload = apply_template(payload_template) if payload_template else {
+ payload = apply_webhook_template(payload_template, template_vars) if payload_template else {In backend/apps/response/notification_dispatcher.py:
+from .template_utils import apply_webhook_template
def _send_to_custom_webhook(self, channel, report_data, mr_info, start_time):
...
- def apply_template(obj):
- if isinstance(obj, str):
- result = obj
- for var, value in template_vars.items():
- result = result.replace(var, value)
- return result
- elif isinstance(obj, dict):
- return {apply_template(k): apply_template(v) for k, v in obj.items()}
- elif isinstance(obj, list):
- return [apply_template(item) for item in obj]
- return obj
-
- payload = apply_template(payload_template) if payload_template else {
+ payload = apply_webhook_template(payload_template, template_vars) if payload_template else {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/apps/response/notification_dispatcher.py` around lines 696 - 706, The
duplicate apply_template implementation should be extracted into a shared
utility: create a new function apply_webhook_template(obj, template_vars) in a
new template_utils module containing the recursive string/dict/list replacement
logic, then replace the local apply_template definitions with an import and call
to apply_webhook_template in both places where apply_template is duplicated
(replace the local function bodies and usages), ensuring the new utility accepts
the same inputs and preserves behavior for str, dict, list, and other types.
Comment on lines
+15
to
+16
| RUN curl -fsSL https://claude.ai/install.sh | bash \ | ||
| && claude --version || echo "WARNING: Claude CLI installation failed" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify the risky chain exists
rg -n 'claude --version\s*\|\|\s*echo' docker/backend/Dockerfile
# Demonstrate shell short-circuit behavior similar to `a && b || c`
bash -lc 'false && true || echo WARNING; echo "final_exit_status=$?"'Repository: mimo-x/Code-Review-GPT-Gitlab
Length of output: 295
Fail the Docker build when Claude CLI install/version check fails
- In
docker/backend/Dockerfile(Lines 15–16), the trailing|| echo "WARNING: Claude CLI installation failed"makes theRUNstep succeed (exit code 0) even whenclaude --versionfails, so a broken install can ship.
Suggested fix
-RUN curl -fsSL https://claude.ai/install.sh | bash \
- && claude --version || echo "WARNING: Claude CLI installation failed"
+RUN if curl -fsSL https://claude.ai/install.sh | bash && claude --version; then \
+ echo "Claude CLI installed"; \
+ else \
+ echo "ERROR: Claude CLI installation failed" >&2; \
+ exit 1; \
+ fi📝 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.
Suggested change
| RUN curl -fsSL https://claude.ai/install.sh | bash \ | |
| && claude --version || echo "WARNING: Claude CLI installation failed" | |
| RUN if curl -fsSL https://claude.ai/install.sh | bash && claude --version; then \ | |
| echo "Claude CLI installed"; \ | |
| else \ | |
| echo "ERROR: Claude CLI installation failed" >&2; \ | |
| exit 1; \ | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker/backend/Dockerfile` around lines 15 - 16, The Dockerfile RUN step
silently succeeds when the Claude CLI install/version check fails because of the
trailing "|| echo ..." — change the RUN so that the install command and the
version check must both succeed (e.g., remove the "|| echo ..." and chain with
&& so "claude --version" returns a non‑zero exit to fail the build) and ensure
the shell in that RUN uses a failing-on-error behavior; additionally, inspect
the init script copied later (the script referenced by the COPY that sets
"project root") and confirm its project-root resolution logic will point to the
intended runtime directory (adjust path resolution in that script if it relies
on build-time paths).
Comment on lines
+19
to
+20
| COPY scripts/init-claude.sh /usr/local/bin/init-claude | ||
| RUN chmod +x /usr/local/bin/init-claude |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Cross-file contract check
rg -n 'COPY scripts/init-claude.sh|SCRIPT_DIR=|PROJECT_ROOT=' docker/backend/Dockerfile scripts/init-claude.sh
# Show what project root resolves to if script lives in /usr/local/bin
bash -lc 'SCRIPT_DIR="/usr/local/bin"; PROJECT_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)"; echo "resolved_project_root=$PROJECT_ROOT"'Repository: mimo-x/Code-Review-GPT-Gitlab
Length of output: 463
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show relevant Dockerfile lines around the COPY
echo "---- docker/backend/Dockerfile: lines 1-60 ----"
nl -ba docker/backend/Dockerfile | sed -n '1,80p'
echo "---- scripts/init-claude.sh: lines 1-200 ----"
nl -ba scripts/init-claude.sh | sed -n '1,220p'
# Search for where PROJECT_ROOT is used
echo "---- usages of PROJECT_ROOT in scripts/init-claude.sh ----"
rg -n 'PROJECT_ROOT' scripts/init-claude.sh
# Search for .env/CLAUDE.md checks
echo "---- checks for .env / CLAUDE.md ----"
rg -n '\.env|CLAUDE\.md' scripts/init-claude.shRepository: mimo-x/Code-Review-GPT-Gitlab
Length of output: 182
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- docker/backend/Dockerfile (first 120 lines) ----"
sed -n '1,120p' docker/backend/Dockerfile
echo "---- scripts/init-claude.sh (first 220 lines) ----"
sed -n '1,220p' scripts/init-claude.sh
echo "---- usages of PROJECT_ROOT in scripts/init-claude.sh ----"
rg -n 'PROJECT_ROOT' scripts/init-claude.sh
echo "---- checks for .env / CLAUDE.md ----"
rg -n '\.env|CLAUDE\.md' scripts/init-claude.shRepository: mimo-x/Code-Review-GPT-Gitlab
Length of output: 9164
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- repo-level CLAUDE.md locations ----"
fd -t f -i "CLAUDE\.md" . || true
echo "---- repo-level .env locations ----"
fd -t f -i "\.env$" . || true
echo "---- show project root related files in backend/ and repo root ----"
ls -la
ls -la backend || trueRepository: mimo-x/Code-Review-GPT-Gitlab
Length of output: 2392
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- rg init-claude usage ----"
rg -n "init-claude" docker-compose.yml docker backend -S || true
echo "---- show docker-compose.yml ----"
sed -n '1,220p' docker-compose.yml
echo "---- list docker/backend contents (small) ----"
find docker/backend -maxdepth 2 -type f -printRepository: mimo-x/Code-Review-GPT-Gitlab
Length of output: 1503
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "init-claude|init\.claude" . || true
rg -n "CLAUDE_CLI_PATH|CLAUDE\.md|PROJECT_ROOT=\$" scripts docker docs README* || trueRepository: mimo-x/Code-Review-GPT-Gitlab
Length of output: 1785
Fix init-claude project-root detection when placed at /usr/local/bin (avoid using /usr/local as root).
docker/backend/Dockerfile copies scripts/init-claude.sh to /usr/local/bin/init-claude; init-claude.sh then derives PROJECT_ROOT from the script’s location ($SCRIPT_DIR/..), so .env/CLAUDE.md checks target /usr/local/.env and /usr/local/CLAUDE.md (and will create /usr/local/.env if missing).
Additionally, the Docker/compose setup never runs init-claude automatically, so this mainly affects manual container usage. The suggested change of moving the script under /app/scripts/ would make PROJECT_ROOT=/app, but this container mounts/copies only backend/ into /app while the repo’s .env and CLAUDE.md are at the repo root (./.env, ./CLAUDE.md), so you still need to copy those files into the image or adjust PROJECT_ROOT logic (e.g., configurable override/flag based on the mounted repo root).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker/backend/Dockerfile` around lines 19 - 20, The init-claude script
deriving PROJECT_ROOT from its install location causes it to treat /usr/local as
the repo root when copied to /usr/local/bin; update the Dockerfile and/or the
script so PROJECT_ROOT correctly points to the repository root: either (A) copy
scripts/init-claude.sh into a path under /app (e.g., /app/scripts/) so
PROJECT_ROOT resolves to /app, and also COPY the repo-level files .env and
CLAUDE.md into the image or the /app location, or (B) change init-claude.sh’s
PROJECT_ROOT logic to prefer an explicit override (ENV var or CLI flag like
INIT_CLAUDE_ROOT) or to detect a mounted repo root (search upward for
.env/CLAUDE.md instead of using $SCRIPT_DIR/..). Update references to
PROJECT_ROOT in init-claude.sh accordingly and ensure the Dockerfile provides
the chosen override or copies the needed repo files.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概述
一次性解决所有 3 个标记为
will_fix的 open issues。#24 [BUG] Review 超时报错修复
_filter_changes文件过滤反转逻辑（excluded 类型实际未生效）LLMConfig新增timeout字段，unionllm/litellm调用增加超时保护（默认 600 秒）review.save()静默吞异常问题#22 通知功能扩展
custom_webhook通用通知类型，支持自定义 URL + HTTP 方法 + JSON 模板变量替换#56 Claude Code 环境初始化
scripts/init-claude.sh一键初始化脚本变更文件
review/services.py,llm/services.py,webhook/views.pyresponse/notification_dispatcher.py,response/services.py,llm/views.pyllm/models.py,llm/serializers.pyfrontend/src/views/Config.vuedocker/backend/Dockerfilescripts/init-claude.sh(新建)0009_add_llmconfig_timeout.py,0010_add_custom_webhook.pySummary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests
Chores