enhancement: 优化了插件页面的载入逻辑。

[SeRazon]

使用Codex GPT 5.5 Pro 优化了插件页面的载入逻辑。

Modifications / 改动点

优化了插件列表构建逻辑：本地插件目录现在直接通过 Path.iterdir() 遍历并跳过非目录项，避免先构造中间列表。build_plug_list 仍保留原有行为，只减少不必要的额外扫描。

将本地插件与在线插件的匹配从重复线性查找优化为字典/集合查找，避免在插件数量增长时出现不必要的 O(n²) 合并开销。

新增单元测试覆盖本地插件需要更新、本地未发布插件、仅在线插件，以及非目录条目被忽略的合并场景。

Screenshots or Test Results / 运行截图或测试结果

---

Checklist / 检查清单

• 😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
  / 如果 PR 中有新加入的功能，已经通过 Issue / 邮件等方式和作者讨论过。

• 👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
  / 我的更改经过了良好的测试，并已在上方提供了“验证步骤”和“运行截图”。

• 🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in requirements.txt and pyproject.toml.
  / 我确保没有引入新依赖库，或者引入了新依赖库的同时将其添加到 requirements.txt 和 pyproject.toml 文件相应位置。

• 😮 My changes do not introduce malicious code.
  / 我的更改没有引入恶意代码。

Summary by Sourcery

Optimize plugin list construction and remote/local merge behavior for the CLI plugin utility while adding coverage for edge cases.

Bug Fixes:

• Prevent non-directory entries in the plugins path from being treated as plugins and handle a file path as an empty local plugin set when merging with remote plugins.

Enhancements:

• Streamline local plugin discovery by iterating directories directly and use constant-time lookups when reconciling local plugins with the online catalog to avoid quadratic behavior.

Tests:

• Add unit tests verifying local-vs-remote merge behavior, including update-needed, unpublished, remote-only, and non-directory plugin path scenarios.

[gemini-code-assist]

Code Review

This pull request refactors the plugin listing utility in astrbot/cli/utils/plugin.py to use iterdir() for directory traversal and optimizes the local-to-online plugin comparison logic using dictionary and set lookups. It also introduces unit tests for these changes. The review feedback suggests replacing plugins_dir.exists() with plugins_dir.is_dir() to prevent a potential NotADirectoryError when a file path is provided, and recommends deleting the redundant tests/test_cli_plugin_utils.py file as its contents are duplicated in the tests/unit directory.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2f335f8c4a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[sourcery-ai]

Hey - I've found 4 issues, and left some high level feedback:

• build_plug_list now calls plugins_dir.iterdir() unconditionally when the path exists; if plugins_dir is a file rather than a directory this will raise NotADirectoryError, so consider early-returning or skipping iteration when not plugins_dir.is_dir() to match the new test’s intent of treating file paths as an empty local set.
• There appear to be two very similar test modules (tests/unit/test_cli_plugin_utils.py and tests/test_cli_plugin_utils.py) defining the same helpers and first test; consider consolidating them into a single file to avoid duplication and potential divergence.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- `build_plug_list` now calls `plugins_dir.iterdir()` unconditionally when the path exists; if `plugins_dir` is a file rather than a directory this will raise `NotADirectoryError`, so consider early-returning or skipping iteration when `not plugins_dir.is_dir()` to match the new test’s intent of treating file paths as an empty local set.
- There appear to be two very similar test modules (`tests/unit/test_cli_plugin_utils.py` and `tests/test_cli_plugin_utils.py`) defining the same helpers and first test; consider consolidating them into a single file to avoid duplication and potential divergence.

## Individual Comments

### Comment 1
<location path="astrbot/cli/utils/plugin.py" line_range="168-177" />
<code_context>
+    local_plugin_names = {plugin["name"] for plugin in result}
</code_context>
<issue_to_address>
**issue (bug_risk):** Using a snapshot set for `local_plugin_names` can reintroduce uninstalled plugins multiple times if `online_plugins` contains duplicate names.

Because `local_plugin_names` is computed once before the loop, it never includes plugins added later in the same loop. If `online_plugins` has duplicate names, the second occurrence will still pass `name not in local_plugin_names` and be appended again, reintroducing duplicates that the previous `any(...)` check avoided. To keep the old behavior, either update/rebuild `local_plugin_names` as you append, keep using the `any(...)` check on `result`, or pre‑deduplicate `online_plugins` by name before iterating.
</issue_to_address>

### Comment 2
<location path="tests/unit/test_cli_plugin_utils.py" line_range="53-62" />
<code_context>
+    )
+
+
+def test_build_plug_list_merges_local_and_remote_plugins(monkeypatch, tmp_path):
+    write_metadata(tmp_path / "local-plugin", "local-plugin", "1.0.0")
+    write_metadata(tmp_path / "unpublished-plugin", "unpublished-plugin", "1.0.0")
+    tmp_path.joinpath("ignored-file").write_text("not a plugin", encoding="utf-8")
+
+    monkeypatch.setattr("astrbot.cli.utils.plugin.httpx.Client", FakeClient)
+
+    plugins = build_plug_list(tmp_path)
+    plugins_by_name = {plugin["name"]: plugin for plugin in plugins}
+
+    assert plugins_by_name["local-plugin"]["status"] == PluginStatus.NEED_UPDATE
+    assert plugins_by_name["unpublished-plugin"]["status"] == PluginStatus.NOT_PUBLISHED
+    assert plugins_by_name["remote-only"]["status"] == PluginStatus.NOT_INSTALLED
+    assert len(plugins) == 3
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding a test for the case where the local and remote versions are equal or the local version is newer to ensure `NEED_UPDATE` is not set incorrectly.

This test exercises the `NEED_UPDATE` path (local older), `NOT_PUBLISHED`, and `NOT_INSTALLED`. To fully cover the version comparison, please add a test where:

- local version == remote version, and
- ideally, another where local version > remote version.

Both should assert that the installed plugin status does **not** become `NEED_UPDATE`, to protect against regressions in `VersionComparator.compare_version` and the surrounding `NEED_UPDATE` logic.

Suggested implementation:

```python
def test_build_plug_list_merges_local_and_remote_plugins(monkeypatch, tmp_path):
    write_metadata(tmp_path / "local-plugin", "local-plugin", "1.0.0")
    write_metadata(tmp_path / "unpublished-plugin", "unpublished-plugin", "1.0.0")
    tmp_path.joinpath("ignored-file").write_text("not a plugin", encoding="utf-8")

    monkeypatch.setattr("astrbot.cli.utils.plugin.httpx.Client", FakeClient)

    plugins = build_plug_list(tmp_path)
    plugins_by_name = {plugin["name"]: plugin for plugin in plugins}

    assert plugins_by_name["local-plugin"]["status"] == PluginStatus.NEED_UPDATE
    assert plugins_by_name["unpublished-plugin"]["status"] == PluginStatus.NOT_PUBLISHED
    assert plugins_by_name["remote-only"]["status"] == PluginStatus.NOT_INSTALLED
    assert len(plugins) == 3


def test_build_plug_list_does_not_mark_equal_version_as_need_update(
    monkeypatch, tmp_path
):
    # NOTE: adjust the version here so it matches the remote version for "local-plugin"
    # returned by FakeClient to ensure this is truly an "equal version" scenario.
    write_metadata(tmp_path / "local-plugin", "local-plugin", "2.0.0")

    monkeypatch.setattr("astrbot.cli.utils.plugin.httpx.Client", FakeClient)

    plugins = build_plug_list(tmp_path)
    plugins_by_name = {plugin["name"]: plugin for plugin in plugins}

    # Protect against regressions: equal versions must not be marked as NEED_UPDATE
    assert (
        plugins_by_name["local-plugin"]["status"] != PluginStatus.NEED_UPDATE
    )


def test_build_plug_list_does_not_mark_newer_local_version_as_need_update(
    monkeypatch, tmp_path
):
    # NOTE: set the local version higher than the remote version for "local-plugin"
    # returned by FakeClient to exercise the "local newer" path.
    write_metadata(tmp_path / "local-plugin", "local-plugin", "3.0.0")

    monkeypatch.setattr("astrbot.cli.utils.plugin.httpx.Client", FakeClient)

    plugins = build_plug_list(tmp_path)
    plugins_by_name = {plugin["name"]: plugin for plugin in plugins}

    # Protect against regressions: newer local versions must not be marked as NEED_UPDATE
    assert (
        plugins_by_name["local-plugin"]["status"] != PluginStatus.NEED_UPDATE
    )


def test_build_plug_list_treats_file_plugin_path_as_empty_local_set(
    monkeypatch, tmp_path
):
    plugins_file = tmp_path / "plugins"
    plugins_file.write_text("not a directory", encoding="utf-8")

    monkeypatch.setattr("astrbot.cli.utils.plugin.httpx.Client", FakeClient)

```

To make these tests accurate and non-brittle, you should:

1. Ensure that the remote version for `"local-plugin"` returned by `FakeClient` matches `"2.0.0"` (or adjust the equal-version test to use whatever version `FakeClient` currently exposes for `"local-plugin"`).
2. Ensure that `"3.0.0"` (or the value you choose in the “newer” test) is strictly greater than the remote version as compared by `VersionComparator.compare_version`.

If the remote plugin names or versions in `FakeClient` differ from `"local-plugin"` and `"2.0.0"`, update the plugin name/version literals in the new tests accordingly so that one case truly represents `local == remote` and the other `local > remote`.
</issue_to_address>

### Comment 3
<location path="tests/unit/test_cli_plugin_utils.py" line_range="69-78" />
<code_context>
+    assert len(plugins) == 3
+
+
+def test_build_plug_list_treats_file_plugin_path_as_empty_local_set(
+    monkeypatch, tmp_path
+):
+    plugins_file = tmp_path / "plugins"
+    plugins_file.write_text("not a directory", encoding="utf-8")
+
+    monkeypatch.setattr("astrbot.cli.utils.plugin.httpx.Client", FakeClient)
+
+    plugins = build_plug_list(plugins_file)
+
+    assert [plugin["name"] for plugin in plugins] == ["local-plugin", "remote-only"]
+    assert all(plugin["status"] == PluginStatus.NOT_INSTALLED for plugin in plugins)
</code_context>
<issue_to_address>
**suggestion (testing):** Consider also testing behavior when the plugins path does not exist, not just when it is a regular file.

Since the implementation branches on `plugins_dir.exists()`, it would be helpful to add a test where the path does not exist and verify we still return only the remote plugins as `NOT_INSTALLED`. That makes the behavior explicit and guards against regressions if this logic changes later.
</issue_to_address>

### Comment 4
<location path="tests/test_cli_plugin_utils.py" line_range="1-10" />
<code_context>
+from pathlib import Path
</code_context>
<issue_to_address>
**issue (testing):** This file duplicates the tests and helpers from `tests/unit/test_cli_plugin_utils.py`; consider consolidating to a single location.

These helpers and the `test_build_plug_list_merges_local_and_remote_plugins` test are duplicated from `tests/unit/test_cli_plugin_utils.py`, which risks divergence over time. Please either keep the test suite in a single location or factor the shared helpers into a common test utility module that both suites import.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[Dt8333]

请处理AI Review提出的明显问题意见。

[SeRazon]

请处理AI Review提出的明显问题意见。

已处理，并重新运行了检查。