fix: install only missing plugin dependencies#6088
Merged
zouyonghe merged 7 commits intoAstrBotDevs:masterfrom Mar 12, 2026
Merged
fix: install only missing plugin dependencies#6088zouyonghe merged 7 commits intoAstrBotDevs:masterfrom
zouyonghe merged 7 commits intoAstrBotDevs:masterfrom
Conversation
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
dosubot
bot
added
size:L
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - 我留下了一些整体性的反馈：
- 在
plan_missing_requirements_install/_install_requirements_with_precheck中，建议处理这样一种情况：missing_names非空但install_lines最终为空（例如依赖项无法干净地映射到具体行）。可以通过显式记录日志，或者退回到安装完整的 requirements 文件来处理，而不是写入并从一个实际上为空的临时文件中安装。 - 在
_install_requirements_with_precheck中，你两次调用了get_astrbot_temp_path()（一次用于os.makedirs，一次作为dir参数）；把返回值先存到一个局部变量中可以避免重复调用，并在该返回路径将来变得非确定性的情况下，避免产生一些微妙的问题。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- In `plan_missing_requirements_install` / `_install_requirements_with_precheck`, consider handling the case where `missing_names` is non-empty but `install_lines` ends up empty (e.g., requirements not mapped cleanly to lines) by either logging explicitly or falling back to installing the full requirements file instead of writing and installing from an effectively empty temp file.
- In `_install_requirements_with_precheck`, you call `get_astrbot_temp_path()` twice (for `os.makedirs` and as the `dir` argument); capturing this in a local variable would avoid redundant calls and prevent subtle issues if the returned path ever becomes non-deterministic.帮我变得更有用！请在每条评论上点按 👍 或 👎，我会根据你的反馈改进后续的评审。
Original comment in English
Hey - I've left some high level feedback:
- In
plan_missing_requirements_install/_install_requirements_with_precheck, consider handling the case wheremissing_namesis non-empty butinstall_linesends up empty (e.g., requirements not mapped cleanly to lines) by either logging explicitly or falling back to installing the full requirements file instead of writing and installing from an effectively empty temp file. - In
_install_requirements_with_precheck, you callget_astrbot_temp_path()twice (foros.makedirsand as thedirargument); capturing this in a local variable would avoid redundant calls and prevent subtle issues if the returned path ever becomes non-deterministic.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `plan_missing_requirements_install` / `_install_requirements_with_precheck`, consider handling the case where `missing_names` is non-empty but `install_lines` ends up empty (e.g., requirements not mapped cleanly to lines) by either logging explicitly or falling back to installing the full requirements file instead of writing and installing from an effectively empty temp file.
- In `_install_requirements_with_precheck`, you call `get_astrbot_temp_path()` twice (for `os.makedirs` and as the `dir` argument); capturing this in a local variable would avoid redundant calls and prevent subtle issues if the returned path ever becomes non-deterministic.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
我们未能获取拉取请求 #6088。
你可以在此拉取请求中评论 @sourcery-ai review 以重试，或者联系我们寻求帮助。
Original comment in English
We failed to fetch pull request #6088.
You can try again by commenting this pull request with @sourcery-ai review, or contact us for help.
Member
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并留下了一些总体反馈：
- 在
_install_requirements_with_precheck中，回退时的日志消息仍然写的是预检查失败，但现在plan_missing_requirements_install在多种非错误场景下都会返回None（例如遇到 option/direct-reference 行或无法映射的缺失依赖名），因此在日志文案中区分真正的预检查失败与“预期的回退条件”会更清晰。 build_missing_requirements_install_lines在遇到 option 或 direct-reference 行时会返回None，但这是静默处理的；可以考虑在这种情况发生时增加一个有针对性的 debug/info 日志，这样在这些情况下为什么会回退到完整 requirements 安装就更容易理解。
给 AI Agent 的提示词
Please address the comments from this code review:
## Overall Comments
- 在 `_install_requirements_with_precheck` 中，回退时的日志消息仍然写的是预检查失败，但现在 `plan_missing_requirements_install` 在多种非错误场景下都会返回 `None`（例如遇到 option/direct-reference 行或无法映射的缺失依赖名），因此在日志文案中区分真正的预检查失败与“预期的回退条件”会更清晰。
- `build_missing_requirements_install_lines` 在遇到 option 或 direct-reference 行时会返回 `None`，但这是静默处理的；可以考虑在这种情况发生时增加一个有针对性的 debug/info 日志，这样在这些情况下为什么会回退到完整 requirements 安装就更容易理解。
## Individual Comments
### Comment 1
<location path="astrbot/core/star/star_manager.py" line_range="102-121" />
<code_context>
)
- await pip_installer.install(requirements_path=requirements_path)
+
+ filtered_requirements_path: str | None = None
+ temp_dir = get_astrbot_temp_path()
+ try:
+ os.makedirs(temp_dir, exist_ok=True)
+ with tempfile.NamedTemporaryFile(
+ mode="w",
+ suffix="_plugin_requirements.txt",
+ delete=False,
+ dir=temp_dir,
+ encoding="utf-8",
+ ) as filtered_requirements_file:
+ filtered_requirements_file.write(
+ "\n".join(install_plan.install_lines) + "\n"
+ )
+ filtered_requirements_path = filtered_requirements_file.name
+
+ await pip_installer.install(requirements_path=filtered_requirements_path)
+ finally:
+ if filtered_requirements_path and os.path.exists(filtered_requirements_path):
+ os.remove(filtered_requirements_path)
</code_context>
<issue_to_address>
**suggestion (bug_risk):** 让临时文件清理在面对竞争删除或文件系统错误时更加健壮。
由于 `finally` 代码块在 `os.path.exists` 之后无条件调用 `os.remove`，竞争条件或文件系统问题可能会抛出 `OSError`，并掩盖真正的安装失败原因。请将 `os.remove` 包装在 `try/except OSError` 中（记录日志或忽略异常），以避免清理失败干扰真实错误的上报。
```suggestion
filtered_requirements_path: str | None = None
temp_dir = get_astrbot_temp_path()
try:
os.makedirs(temp_dir, exist_ok=True)
with tempfile.NamedTemporaryFile(
mode="w",
suffix="_plugin_requirements.txt",
delete=False,
dir=temp_dir,
encoding="utf-8",
) as filtered_requirements_file:
filtered_requirements_file.write(
"\n".join(install_plan.install_lines) + "\n"
)
filtered_requirements_path = filtered_requirements_file.name
await pip_installer.install(requirements_path=filtered_requirements_path)
finally:
if filtered_requirements_path and os.path.exists(filtered_requirements_path):
try:
os.remove(filtered_requirements_path)
except OSError as e:
logger.warning(
"删除临时插件依赖文件失败：%s（路径：%s）",
e,
filtered_requirements_path,
)
```
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据这些反馈改进后续的代码审查。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- In
_install_requirements_with_precheck, the fallback log message still says the precheck failed, butplan_missing_requirements_installnow returnsNonefor several non-error cases (e.g., option/direct-reference lines or unmapped missing names), so it would be clearer to distinguish actual precheck failure from intentional fallback conditions in the log text. build_missing_requirements_install_linesreturnsNonewhen encountering option or direct-reference lines but does so silently; consider adding a targeted debug/info log when this happens so it’s easier to understand why the code fell back to full requirements installation in those situations.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `_install_requirements_with_precheck`, the fallback log message still says the precheck failed, but `plan_missing_requirements_install` now returns `None` for several non-error cases (e.g., option/direct-reference lines or unmapped missing names), so it would be clearer to distinguish actual precheck failure from intentional fallback conditions in the log text.
- `build_missing_requirements_install_lines` returns `None` when encountering option or direct-reference lines but does so silently; consider adding a targeted debug/info log when this happens so it’s easier to understand why the code fell back to full requirements installation in those situations.
## Individual Comments
### Comment 1
<location path="astrbot/core/star/star_manager.py" line_range="102-121" />
<code_context>
)
- await pip_installer.install(requirements_path=requirements_path)
+
+ filtered_requirements_path: str | None = None
+ temp_dir = get_astrbot_temp_path()
+ try:
+ os.makedirs(temp_dir, exist_ok=True)
+ with tempfile.NamedTemporaryFile(
+ mode="w",
+ suffix="_plugin_requirements.txt",
+ delete=False,
+ dir=temp_dir,
+ encoding="utf-8",
+ ) as filtered_requirements_file:
+ filtered_requirements_file.write(
+ "\n".join(install_plan.install_lines) + "\n"
+ )
+ filtered_requirements_path = filtered_requirements_file.name
+
+ await pip_installer.install(requirements_path=filtered_requirements_path)
+ finally:
+ if filtered_requirements_path and os.path.exists(filtered_requirements_path):
+ os.remove(filtered_requirements_path)
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Make temp file cleanup more robust against removal races or filesystem errors.
Because the `finally` block unconditionally calls `os.remove` after `os.path.exists`, a race or filesystem issue could raise an `OSError` and hide the original install failure. Wrap `os.remove` in a `try/except OSError` (logging or ignoring the exception) so cleanup failures don’t interfere with reporting the real error.
```suggestion
filtered_requirements_path: str | None = None
temp_dir = get_astrbot_temp_path()
try:
os.makedirs(temp_dir, exist_ok=True)
with tempfile.NamedTemporaryFile(
mode="w",
suffix="_plugin_requirements.txt",
delete=False,
dir=temp_dir,
encoding="utf-8",
) as filtered_requirements_file:
filtered_requirements_file.write(
"\n".join(install_plan.install_lines) + "\n"
)
filtered_requirements_path = filtered_requirements_file.name
await pip_installer.install(requirements_path=filtered_requirements_path)
finally:
if filtered_requirements_path and os.path.exists(filtered_requirements_path):
try:
os.remove(filtered_requirements_path)
except OSError as e:
logger.warning(
"删除临时插件依赖文件失败：%s（路径：%s）",
e,
filtered_requirements_path,
)
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
dosubot
bot
added
size:XL
Member
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - 我在这里给出了一些整体性的反馈：
_install_requirements_with_precheck中对临时过滤后依赖文件的处理（文件创建、日志记录和清理）现在逻辑有点复杂；可以考虑把这部分提取成一个小的帮助函数或上下文管理器，这样主流程会更清晰，错误处理/清理逻辑也更容易理解和复用。- 若干检查日志输出的测试（例如
test_build_missing_requirements_install_lines_logs_why_option_lines_fall_back和test_find_missing_requirements_logs_path_and_reason_on_precheck_fallback）是通过手动 monkeypatchlogger.*来实现的；使用caplog会让这些测试更简单，也能降低对具体 logger 实现的耦合度。 test_plugin_manager.py中新增的一些测试辅助函数（_build_dependency_install_*_mock、_mock_missing_requirements_plan、_assert_dependency_install_event_matches等）在行为和参数上有一定重叠；可以考虑将它们整合，或通过参数化的方式抽取公共行为，以减少重复，并让每个测试的意图更加清晰。
面向 AI 代理的提示语
Please address the comments from this code review:
## Overall Comments
- The temporary filtered requirements handling in `_install_requirements_with_precheck` (file creation, logging, cleanup) is now somewhat complex; consider extracting this into a small helper/context manager so the main flow reads more clearly and error/cleanup behavior is easier to reason about and reuse.
- Several tests that inspect logging (e.g., in `test_build_missing_requirements_install_lines_logs_why_option_lines_fall_back` and `test_find_missing_requirements_logs_path_and_reason_on_precheck_fallback`) manually monkeypatch `logger.*`; using `caplog` would simplify these tests and make them less coupled to the logger implementation.
- The new test helpers in `test_plugin_manager.py` (`_build_dependency_install_*_mock`, `_mock_missing_requirements_plan`, `_assert_dependency_install_event_matches`, etc.) overlap in behavior and signatures; consider consolidating them or parameterizing common behavior to reduce duplication and make the intent of each test clearer.帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续的评审。
Original comment in English
Hey - I've left some high level feedback:
- The temporary filtered requirements handling in
_install_requirements_with_precheck(file creation, logging, cleanup) is now somewhat complex; consider extracting this into a small helper/context manager so the main flow reads more clearly and error/cleanup behavior is easier to reason about and reuse. - Several tests that inspect logging (e.g., in
test_build_missing_requirements_install_lines_logs_why_option_lines_fall_backandtest_find_missing_requirements_logs_path_and_reason_on_precheck_fallback) manually monkeypatchlogger.*; usingcaplogwould simplify these tests and make them less coupled to the logger implementation. - The new test helpers in
test_plugin_manager.py(_build_dependency_install_*_mock,_mock_missing_requirements_plan,_assert_dependency_install_event_matches, etc.) overlap in behavior and signatures; consider consolidating them or parameterizing common behavior to reduce duplication and make the intent of each test clearer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The temporary filtered requirements handling in `_install_requirements_with_precheck` (file creation, logging, cleanup) is now somewhat complex; consider extracting this into a small helper/context manager so the main flow reads more clearly and error/cleanup behavior is easier to reason about and reuse.
- Several tests that inspect logging (e.g., in `test_build_missing_requirements_install_lines_logs_why_option_lines_fall_back` and `test_find_missing_requirements_logs_path_and_reason_on_precheck_fallback`) manually monkeypatch `logger.*`; using `caplog` would simplify these tests and make them less coupled to the logger implementation.
- The new test helpers in `test_plugin_manager.py` (`_build_dependency_install_*_mock`, `_mock_missing_requirements_plan`, `_assert_dependency_install_event_matches`, etc.) overlap in behavior and signatures; consider consolidating them or parameterizing common behavior to reduce duplication and make the intent of each test clearer.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
SourceryAI
reviewed
Mar 12, 2026
SourceryAI
left a comment
SourceryAI
left a comment
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并留下了一些整体性的反馈：
- 现在有两个辅助函数实际上在做同样的事情（
_mock_precheck_fails和_mock_precheck_plan_failure都通过 monkeypatch 将plan_missing_requirements_install修改为返回None）；建议合并为一个辅助函数，或者删除未使用的那个，以避免混淆。 - 在
plan_missing_requirements_install中，None这个返回值把“无法进行预检查”（例如包含选项/直接引用的行）与find_missing_requirements返回的真实“没有缺失依赖”结果混为一谈；建议对于选项/直接引用这种情况返回一个带有明确fallback_reason的MissingRequirementsPlan，这样调用方和日志就能更清晰地区分这些场景。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There are now two helpers that effectively do the same thing (`_mock_precheck_fails` and `_mock_precheck_plan_failure` both monkeypatch `plan_missing_requirements_install` to return `None`); consider consolidating to a single helper or removing the unused one to avoid confusion.
- In `plan_missing_requirements_install`, the `None` return value conflates the case where precheck is impossible (e.g., option/direct-reference lines) with a genuine 'no missing deps' result from `find_missing_requirements`; consider returning a `MissingRequirementsPlan` with an explicit `fallback_reason` for the option/direct-reference case so callers and logs can distinguish these scenarios more clearly.
## Individual Comments
### Comment 1
<location path="tests/test_pip_helper_modules.py" line_range="210-219" />
<code_context>
+ assert install_lines == ("boto3",)
+
+
+def test_plan_missing_requirements_install_returns_none_when_missing_names_cannot_map_to_lines(
+ monkeypatch,
+ tmp_path,
+):
+ requirements_path = tmp_path / "requirements.txt"
+ requirements_path.write_text("boto3\n", encoding="utf-8")
+
+ monkeypatch.setattr(
+ requirements_utils,
+ "find_missing_requirements",
+ lambda path: {"botocore"},
+ )
+
+ plan = requirements_utils.plan_missing_requirements_install(str(requirements_path))
+
+ assert plan is not None
+ assert plan.missing_names == frozenset({"botocore"})
+ assert plan.install_lines == ()
+ assert plan.fallback_reason == "unmapped missing requirement names"
+
+
</code_context>
<issue_to_address>
**issue (testing):** Test name does not match behavior being asserted for `plan_missing_requirements_install`
The assertions verify a non-`None` plan with empty `install_lines` and a `fallback_reason`, but the test name claims it returns `None`. This mismatch makes the test misleading. Please either rename the test (e.g. `test_plan_missing_requirements_install_returns_fallback_plan_when_names_cannot_map_to_lines`) or change the assertions if the expected behavior is actually `None`.
</issue_to_address>Hi @zouyonghe! 👋
感谢你通过评论 @sourcery-ai review 来试用 Sourcery！🚀
安装 sourcery-ai bot 即可在每个 pull request 上自动获取代码审查 ✨
帮我变得更有用！请在每条评论上点击 👍 或 👎，我会根据你的反馈改进之后的评审。Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- There are now two helpers that effectively do the same thing (
_mock_precheck_failsand_mock_precheck_plan_failureboth monkeypatchplan_missing_requirements_installto returnNone); consider consolidating to a single helper or removing the unused one to avoid confusion. - In
plan_missing_requirements_install, theNonereturn value conflates the case where precheck is impossible (e.g., option/direct-reference lines) with a genuine 'no missing deps' result fromfind_missing_requirements; consider returning aMissingRequirementsPlanwith an explicitfallback_reasonfor the option/direct-reference case so callers and logs can distinguish these scenarios more clearly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There are now two helpers that effectively do the same thing (`_mock_precheck_fails` and `_mock_precheck_plan_failure` both monkeypatch `plan_missing_requirements_install` to return `None`); consider consolidating to a single helper or removing the unused one to avoid confusion.
- In `plan_missing_requirements_install`, the `None` return value conflates the case where precheck is impossible (e.g., option/direct-reference lines) with a genuine 'no missing deps' result from `find_missing_requirements`; consider returning a `MissingRequirementsPlan` with an explicit `fallback_reason` for the option/direct-reference case so callers and logs can distinguish these scenarios more clearly.
## Individual Comments
### Comment 1
<location path="tests/test_pip_helper_modules.py" line_range="210-219" />
<code_context>
+ assert install_lines == ("boto3",)
+
+
+def test_plan_missing_requirements_install_returns_none_when_missing_names_cannot_map_to_lines(
+ monkeypatch,
+ tmp_path,
+):
+ requirements_path = tmp_path / "requirements.txt"
+ requirements_path.write_text("boto3\n", encoding="utf-8")
+
+ monkeypatch.setattr(
+ requirements_utils,
+ "find_missing_requirements",
+ lambda path: {"botocore"},
+ )
+
+ plan = requirements_utils.plan_missing_requirements_install(str(requirements_path))
+
+ assert plan is not None
+ assert plan.missing_names == frozenset({"botocore"})
+ assert plan.install_lines == ()
+ assert plan.fallback_reason == "unmapped missing requirement names"
+
+
</code_context>
<issue_to_address>
**issue (testing):** Test name does not match behavior being asserted for `plan_missing_requirements_install`
The assertions verify a non-`None` plan with empty `install_lines` and a `fallback_reason`, but the test name claims it returns `None`. This mismatch makes the test misleading. Please either rename the test (e.g. `test_plan_missing_requirements_install_returns_fallback_plan_when_names_cannot_map_to_lines`) or change the assertions if the expected behavior is actually `None`.
</issue_to_address>Hi @zouyonghe! 👋
Thanks for trying out Sourcery by commenting with @sourcery-ai review! 🚀
Install the sourcery-ai bot to get automatic code reviews on every pull request ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
tests/test_pip_helper_modules.py
Outdated
Comment on lines
+210
to
+219
| def test_plan_missing_requirements_install_returns_none_when_missing_names_cannot_map_to_lines( | ||
| monkeypatch, | ||
| tmp_path, | ||
| ): | ||
| requirements_path = tmp_path / "requirements.txt" | ||
| requirements_path.write_text("boto3\n", encoding="utf-8") | ||
|
|
||
| monkeypatch.setattr( | ||
| requirements_utils, | ||
| "find_missing_requirements", |
There was a problem hiding this comment.
issue (testing): 测试名称与针对 plan_missing_requirements_install 所断言的行为不一致
断言验证的是一个非 None 的 plan，且其 install_lines 为空，并带有一个 fallback_reason，但测试名称却声称它返回 None。这种不匹配会使测试产生误导。请重命名测试（例如 test_plan_missing_requirements_install_returns_fallback_plan_when_names_cannot_map_to_lines），或者如果期望行为确实是返回 None，则相应地修改断言。
Original comment in English
issue (testing): Test name does not match behavior being asserted for plan_missing_requirements_install
The assertions verify a non-None plan with empty install_lines and a fallback_reason, but the test name claims it returns None. This mismatch makes the test misleading. Please either rename the test (e.g. test_plan_missing_requirements_install_returns_fallback_plan_when_names_cannot_map_to_lines) or change the assertions if the expected behavior is actually None.
Member
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - 我发现了 1 个问题
给 AI Agent 的提示词
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="astrbot/core/utils/requirements_utils.py" line_range="411" />
<code_context>
return missing
+def build_missing_requirements_install_lines(
+ requirements_path: str,
+ missing_names: set[str] | frozenset[str],
</code_context>
<issue_to_address>
**issue (complexity):** 请考虑重构新的缺失依赖规划流程，使依赖只预检查一次并在各个辅助函数之间共享，而不是让每个函数各自重复加载。
你可以在每次规划调用时只加载/预检查一次 requirements 并复用结果，而不是让 `find_missing_requirements()` 和 `build_missing_requirements_install_lines()` 各自独立调用 `_load_requirement_lines_for_precheck`，从而避免大部分新引入的复杂度。
通过少量重构，可以在保持全部现有行为和公共 API 不变的前提下，减少重复逻辑并让控制流更连贯。
### 1. 让 `build_missing_requirements_install_lines` 复用已加载的行
将 `build_missing_requirements_install_lines` 改为接收 `requirement_lines`，而不是再次调用 `_load_requirement_lines_for_precheck`：
```python
def build_missing_requirements_install_lines(
requirements_path: str,
requirement_lines: Sequence[str],
missing_names: set[str] | frozenset[str],
) -> tuple[str, ...] | None:
wanted_names = set(missing_names)
install_lines: list[str] = []
for line in requirement_lines:
parsed = _parse_requirement_line(line)
if parsed is None:
if looks_like_direct_reference(line) or line.startswith(("-", "--")):
logger.debug(
"缺失依赖行筛选回退到完整安装：requirements 中包含无法安全裁剪的 "
"option/direct-reference 行: %s (%s)",
requirements_path,
line,
)
return None
continue
name, _specifier = parsed
if name in wanted_names:
install_lines.append(line)
return tuple(install_lines)
```
### 2. 在 `plan_missing_requirements_install` 中集中规划流程
让 `plan_missing_requirements_install` 只执行一次预检查，并同时为缺失依赖名检测和安装行构建提供数据：
```python
def plan_missing_requirements_install(
requirements_path: str,
) -> MissingRequirementsPlan | None:
can_precheck, requirement_lines = _load_requirement_lines_for_precheck(
requirements_path
)
if not can_precheck or requirement_lines is None:
return None
missing = find_missing_requirements_from_lines(requirement_lines)
if missing is None:
return None
install_lines = build_missing_requirements_install_lines(
requirements_path, requirement_lines, missing
)
if install_lines is None:
return None
if missing and not install_lines:
logger.warning(
"预检查缺失依赖成功，但无法映射到可安装 requirement 行，将回退到完整安装: %s -> %s",
requirements_path,
sorted(missing),
)
return MissingRequirementsPlan(
missing_names=frozenset(missing),
install_lines=(),
fallback_reason="unmapped missing requirement names",
)
return MissingRequirementsPlan(
missing_names=frozenset(missing),
install_lines=install_lines,
)
```
你需要一个小的辅助函数，用来在不重新加载的前提下，基于已加载的行计算缺失依赖名，并复用现有逻辑：
```python
def find_missing_requirements_from_lines(
requirement_lines: Sequence[str],
) -> set[str]:
parsed_input = parse_requirement_lines(requirement_lines)
installed = get_installed_distributions() # or however you obtain this
missing: set[str] = set()
for name, specifier in parsed_input.requirement_names:
installed_version = installed.get(name)
if not installed_version or (specifier and
not _specifier_contains_version(specifier, installed_version)):
missing.add(name)
return missing
```
### 3. 通过委托保留现有公共 API
现有对 `find_missing_requirements()` 的调用可以继续工作，只需通过新的共享路径进行委托：
```python
def find_missing_requirements(requirements_path: str) -> set[str] | None:
can_precheck, requirement_lines = _load_requirement_lines_for_precheck(
requirements_path
)
if not can_precheck or requirement_lines is None:
return None
return find_missing_requirements_from_lines(requirement_lines)
```
这样可以保持全部功能不变，但：
- `_load_requirement_lines_for_precheck` 在一次规划调用中只会运行一次。
- “是否可以裁剪？” 的逻辑集中在一条流程中（`plan_missing_requirements_install` + `build_missing_requirements_install_lines`）。
- 控制流更简单：一次预检查，一次遍历得出缺失依赖名，一次遍历基于相同数据得出安装行。
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进审查质量。
Original comment in English
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="astrbot/core/utils/requirements_utils.py" line_range="411" />
<code_context>
return missing
+def build_missing_requirements_install_lines(
+ requirements_path: str,
+ missing_names: set[str] | frozenset[str],
</code_context>
<issue_to_address>
**issue (complexity):** Consider restructuring the new missing-requirements planning flow so that requirements are prechecked once and shared across helpers instead of each function reloading them independently.
You can avoid most of the new complexity by loading/prechecking the requirements once per planning call and reusing the result, instead of having both `find_missing_requirements()` and `build_missing_requirements_install_lines()` independently call `_load_requirement_lines_for_precheck`.
A small restructuring keeps all behavior and public APIs while reducing duplication and control‑flow fragmentation.
### 1. Let `build_missing_requirements_install_lines` reuse already-loaded lines
Change `build_missing_requirements_install_lines` to accept `requirement_lines` instead of calling `_load_requirement_lines_for_precheck` again:
```python
def build_missing_requirements_install_lines(
requirements_path: str,
requirement_lines: Sequence[str],
missing_names: set[str] | frozenset[str],
) -> tuple[str, ...] | None:
wanted_names = set(missing_names)
install_lines: list[str] = []
for line in requirement_lines:
parsed = _parse_requirement_line(line)
if parsed is None:
if looks_like_direct_reference(line) or line.startswith(("-", "--")):
logger.debug(
"缺失依赖行筛选回退到完整安装：requirements 中包含无法安全裁剪的 "
"option/direct-reference 行: %s (%s)",
requirements_path,
line,
)
return None
continue
name, _specifier = parsed
if name in wanted_names:
install_lines.append(line)
return tuple(install_lines)
```
### 2. Centralize the planning flow in `plan_missing_requirements_install`
Have `plan_missing_requirements_install` perform the precheck once and feed both missing‑name detection and install‑line building:
```python
def plan_missing_requirements_install(
requirements_path: str,
) -> MissingRequirementsPlan | None:
can_precheck, requirement_lines = _load_requirement_lines_for_precheck(
requirements_path
)
if not can_precheck or requirement_lines is None:
return None
missing = find_missing_requirements_from_lines(requirement_lines)
if missing is None:
return None
install_lines = build_missing_requirements_install_lines(
requirements_path, requirement_lines, missing
)
if install_lines is None:
return None
if missing and not install_lines:
logger.warning(
"预检查缺失依赖成功，但无法映射到可安装 requirement 行，将回退到完整安装: %s -> %s",
requirements_path,
sorted(missing),
)
return MissingRequirementsPlan(
missing_names=frozenset(missing),
install_lines=(),
fallback_reason="unmapped missing requirement names",
)
return MissingRequirementsPlan(
missing_names=frozenset(missing),
install_lines=install_lines,
)
```
You’d need a small helper to compute missing names from already‑loaded lines, reusing the existing logic but without reloading:
```python
def find_missing_requirements_from_lines(
requirement_lines: Sequence[str],
) -> set[str]:
parsed_input = parse_requirement_lines(requirement_lines)
installed = get_installed_distributions() # or however you obtain this
missing: set[str] = set()
for name, specifier in parsed_input.requirement_names:
installed_version = installed.get(name)
if not installed_version or (specifier and
not _specifier_contains_version(specifier, installed_version)):
missing.add(name)
return missing
```
### 3. Keep existing public APIs by delegating
Existing callers of `find_missing_requirements()` can continue to work by delegating through the new shared path:
```python
def find_missing_requirements(requirements_path: str) -> set[str] | None:
can_precheck, requirement_lines = _load_requirement_lines_for_precheck(
requirements_path
)
if not can_precheck or requirement_lines is None:
return None
return find_missing_requirements_from_lines(requirement_lines)
```
This keeps all functionality intact, but:
- `_load_requirement_lines_for_precheck` runs only once per planning call.
- The “can we trim?” logic is localized in a single flow (`plan_missing_requirements_install` + `build_missing_requirements_install_lines`).
- The control flow is simpler: one precheck, one pass to derive missing names, one pass to derive install lines from the same data.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
|
@sourcery-ai review |
SourceryAI
reviewed
Mar 12, 2026
SourceryAI
left a comment
SourceryAI
left a comment
There was a problem hiding this comment.
Hey - 我已经审查了你的更改，整体看起来很不错！
Hi @zouyonghe! 👋
感谢你通过评论 @sourcery-ai review 来体验 Sourcery！🚀
安装 sourcery-ai 机器人，即可在每个拉取请求上获得自动代码审查 ✨
帮我变得更有用吧！请在每条评论上点 👍 或 👎，我会根据你的反馈改进我的审查。Original comment in English
Hey - I've reviewed your changes and they look great!
Hi @zouyonghe! 👋
Thanks for trying out Sourcery by commenting with @sourcery-ai review! 🚀
Install the sourcery-ai bot to get automatic code reviews on every pull request ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
There was a problem hiding this comment.
嗨，我已经审查过你的修改了，看起来很棒！
帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈来改进之后的评审。
Original comment in English
Hey - I've reviewed your changes and they look great!
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
stevessr
pushed a commit
to stevessr/AstrBot
that referenced
this pull request
Mar 12, 2026
* chore: ignore local worktrees * fix: install only missing plugin dependencies * fix: harden missing dependency install fallback * fix: clarify dependency install fallback logging * refactor: simplify dependency install test helpers * refactor: reuse requirements precheck planning
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
2 participants
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.
Summary
Verification
uv run ruff check astrbot/core/utils/requirements_utils.py astrbot/core/star/star_manager.py tests/test_pip_helper_modules.py tests/test_plugin_manager.py tests/test_pip_installer.pyuv run pytest tests/test_pip_helper_modules.py tests/test_plugin_manager.py tests/test_pip_installer.py -qSummary by Sourcery
使用预检查计划安装插件依赖，只筛选并安装缺失的依赖行；当无法安全筛选时，可靠地回退到使用完整的 requirements 文件进行安装。
Bug 修复：
增强：
MissingRequirementsPlan抽象，根据预检查结果驱动对插件依赖的筛选式安装。测试：
Original summary in English
Summary by Sourcery
Install plugin dependencies using a precheck plan that filters and installs only missing requirement lines, while safely falling back to full requirements files when filtering is not possible.
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
使用预检查计划安装插件依赖，只筛选并安装缺失的依赖行；当无法安全筛选时，可靠地回退到使用完整的 requirements 文件进行安装。
Bug 修复：
增强：
MissingRequirementsPlan抽象，根据预检查结果驱动对插件依赖的筛选式安装。测试：
Original summary in English
Summary by Sourcery
Install plugin dependencies using a precheck plan that filters and installs only missing requirement lines, while safely falling back to full requirements files when filtering is not possible.
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
使用预检查计划安装插件依赖，只筛选并安装缺失的依赖行；当无法安全筛选时，可靠地回退到使用完整的 requirements 文件进行安装。
Bug 修复：
增强：
MissingRequirementsPlan抽象，根据预检查结果驱动对插件依赖的筛选式安装。测试：
Original summary in English
Summary by Sourcery
Install plugin dependencies using a precheck plan that filters and installs only missing requirement lines, while safely falling back to full requirements files when filtering is not possible.
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
使用预检查计划安装插件依赖，只筛选并安装缺失的依赖行；当无法安全筛选时，可靠地回退到使用完整的 requirements 文件进行安装。
Bug 修复：
增强：
MissingRequirementsPlan抽象，根据预检查结果驱动对插件依赖的筛选式安装。测试：
Original summary in English
Summary by Sourcery
Install plugin dependencies using a precheck plan that filters and installs only missing requirement lines, while safely falling back to full requirements files when filtering is not possible.
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
使用预检查计划安装插件依赖，只筛选并安装缺失的依赖行；当无法安全筛选时，可靠地回退到使用完整的 requirements 文件进行安装。
Bug 修复：
增强：
MissingRequirementsPlan抽象，根据预检查结果驱动对插件依赖的筛选式安装。测试：
Original summary in English
Summary by Sourcery
Install plugin dependencies using a precheck plan that filters and installs only missing requirement lines, while safely falling back to full requirements files when filtering is not possible.
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
使用预检查计划安装插件依赖，只筛选并安装缺失的依赖行；当无法安全筛选时，可靠地回退到使用完整的 requirements 文件进行安装。
Bug 修复：
增强：
MissingRequirementsPlan抽象，根据预检查结果驱动对插件依赖的筛选式安装。测试：
Original summary in English
Summary by Sourcery
Install plugin dependencies using a precheck plan that filters and installs only missing requirement lines, while safely falling back to full requirements files when filtering is not possible.
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
使用预检查计划安装插件依赖，只筛选并安装缺失的依赖行；当无法安全筛选时，可靠地回退到使用完整的 requirements 文件进行安装。
Bug 修复：
增强：
MissingRequirementsPlan抽象，根据预检查结果驱动对插件依赖的筛选式安装。测试：
Original summary in English
Summary by Sourcery
Install plugin dependencies using a precheck plan that filters and installs only missing requirement lines, while safely falling back to full requirements files when filtering is not possible.
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
使用预检查计划安装插件依赖，只筛选并安装缺失的依赖行；当无法安全筛选时，可靠地回退到使用完整的 requirements 文件进行安装。
Bug 修复：
增强：
MissingRequirementsPlan抽象，根据预检查结果驱动对插件依赖的筛选式安装。测试：
Original summary in English
Summary by Sourcery
Install plugin dependencies using a precheck plan that filters and installs only missing requirement lines, while safely falling back to full requirements files when filtering is not possible.
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
使用预检查计划安装插件依赖，只筛选并安装缺失的依赖行；当无法安全筛选时，可靠地回退到使用完整的 requirements 文件进行安装。
Bug 修复：
增强：
MissingRequirementsPlan抽象，根据预检查结果驱动对插件依赖的筛选式安装。测试：
Original summary in English
Summary by Sourcery
Install plugin dependencies using a precheck plan that filters and installs only missing requirement lines, while safely falling back to full requirements files when filtering is not possible.
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
使用预检查计划安装插件依赖，只筛选并安装缺失的依赖行；当无法安全筛选时，可靠地回退到使用完整的 requirements 文件进行安装。
Bug 修复：
增强：
MissingRequirementsPlan抽象，根据预检查结果驱动对插件依赖的筛选式安装。测试：
Original summary in English
Summary by Sourcery
Install plugin dependencies using a precheck plan that filters and installs only missing requirement lines, while safely falling back to full requirements files when filtering is not possible.
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
使用预检查计划安装插件依赖，只筛选并安装缺失的依赖行；当无法安全筛选时，可靠地回退到使用完整的 requirements 文件进行安装。
Bug 修复：
增强：
MissingRequirementsPlan抽象，根据预检查结果驱动对插件依赖的筛选式安装。测试：
Original summary in English
Summary by Sourcery
Install plugin dependencies using a precheck plan that filters and installs only missing requirement lines, while safely falling back to full requirements files when filtering is not possible.
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
使用预检查计划安装插件依赖，只筛选并安装缺失的依赖行；当无法安全筛选时，可靠地回退到使用完整的 requirements 文件进行安装。
Bug 修复：
增强：
MissingRequirementsPlan抽象，根据预检查结果驱动对插件依赖的筛选式安装。测试：
Original summary in English
Summary by Sourcery
Install plugin dependencies using a precheck plan that filters and installs only missing requirement lines, while safely falling back to full requirements files when filtering is not possible.
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
使用预检查计划安装插件依赖，只筛选并安装缺失的依赖行；当无法安全筛选时，可靠地回退到使用完整的 requirements 文件进行安装。
Bug 修复：
增强：
MissingRequirementsPlan抽象，根据预检查结果驱动对插件依赖的筛选式安装。测试：
Original summary in English
Summary by Sourcery
Install plugin dependencies using a precheck plan that filters and installs only missing requirement lines, while safely falling back to full requirements files when filtering is not possible.
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
使用预检查计划安装插件依赖，只筛选并安装缺失的依赖行；当无法安全筛选时，可靠地回退到使用完整的 requirements 文件进行安装。
Bug 修复：
增强：
MissingRequirementsPlan抽象，根据预检查结果驱动对插件依赖的筛选式安装。测试：
Original summary in English
Summary by Sourcery
Install plugin dependencies using a precheck plan that filters and installs only missing requirement lines, while safely falling back to full requirements files when filtering is not possible.
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
使用预检查计划安装插件依赖，只筛选并安装缺失的依赖行；当无法安全筛选时，可靠地回退到使用完整的 requirements 文件进行安装。
Bug 修复：
增强：
MissingRequirementsPlan抽象，根据预检查结果驱动对插件依赖的筛选式安装。测试：
Original summary in English
Summary by Sourcery
Install plugin dependencies using a precheck plan that filters and installs only missing requirement lines, while safely falling back to full requirements files when filtering is not possible.
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
使用预检查计划安装插件依赖，只筛选并安装缺失的依赖行；当无法安全筛选时，可靠地回退到使用完整的 requirements 文件进行安装。
Bug 修复：
增强：
MissingRequirementsPlan抽象，根据预检查结果驱动对插件依赖的筛选式安装。测试：
Original summary in English
Summary by Sourcery
Install plugin dependencies using a precheck plan that filters and installs only missing requirement lines, while safely falling back to full requirements files when filtering is not possible.
Bug Fixes:
Enhancements:
Tests: