Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 27 additions & 34 deletions astrbot/cli/utils/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,10 @@ def build_plug_list(plugins_dir: Path) -> list:
"""
# Get local plugin info
result = []
if plugins_dir.exists():
for plugin_name in [d.name for d in plugins_dir.glob("*") if d.is_dir()]:
plugin_dir = plugins_dir / plugin_name
if plugins_dir.is_dir():
for plugin_dir in plugins_dir.iterdir():
Comment thread
SeRazon marked this conversation as resolved.
if not plugin_dir.is_dir():
continue

# Load metadata from metadata.yaml
metadata = load_yaml_metadata(plugin_dir)
Expand All @@ -141,55 +142,47 @@ def build_plug_list(plugins_dir: Path) -> list:
)

# Get online plugin list
online_plugins = []
online_plugins_dict = {}
try:
with httpx.Client() as client:
resp = client.get("https://api.soulter.top/astrbot/plugins")
resp.raise_for_status()
data = resp.json()
for plugin_id, plugin_info in data.items():
online_plugins.append(
{
"name": str(plugin_id),
"desc": str(plugin_info.get("desc", "")),
"version": str(plugin_info.get("version", "")),
"author": str(plugin_info.get("author", "")),
"repo": str(plugin_info.get("repo", "")),
"status": PluginStatus.NOT_INSTALLED,
"local_path": None,
},
)
online_plugins_dict[str(plugin_id)] = {
"name": str(plugin_id),
"desc": str(plugin_info.get("desc", "")),
"version": str(plugin_info.get("version", "")),
"author": str(plugin_info.get("author", "")),
"repo": str(plugin_info.get("repo", "")),
"status": PluginStatus.NOT_INSTALLED,
"local_path": None,
}
except Exception as e:
click.echo(f"Failed to get online plugin list: {e}", err=True)

# Compare with online plugins and update status
online_plugin_names = {plugin["name"] for plugin in online_plugins}
for local_plugin in result:
Comment thread
SeRazon marked this conversation as resolved.
if local_plugin["name"] in online_plugin_names:
# Find the corresponding online plugin
online_plugin = next(
p for p in online_plugins if p["name"] == local_plugin["name"]
)
if (
VersionComparator.compare_version(
local_plugin["version"],
online_plugin["version"],
)
< 0
):
local_plugin["status"] = PluginStatus.NEED_UPDATE
else:
online_plugin = online_plugins_dict.pop(local_plugin["name"], None)
if online_plugin is None:
# Local plugin is not published online
local_plugin["status"] = PluginStatus.NOT_PUBLISHED
continue

if (
VersionComparator.compare_version(
local_plugin["version"],
online_plugin["version"],
)
< 0
):
local_plugin["status"] = PluginStatus.NEED_UPDATE

# Add uninstalled online plugins
for online_plugin in online_plugins:
if not any(plugin["name"] == online_plugin["name"] for plugin in result):
result.append(online_plugin)
result.extend(online_plugins_dict.values())
Comment on lines +145 to +182

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

当获取在线插件列表失败(例如网络连接问题或 API 服务不可用)时,online_plugins_dict 将为空。在这种情况下,所有本地已安装的插件都会因为 online_plugin is None 而被错误地标记为 PluginStatus.NOT_PUBLISHED(未发布),并在 CLI 界面中显示在“未发布插件”分类下。这会给用户带来误导。

建议引入一个 online_fetched 标志。只有在成功获取在线插件列表时,才将未匹配的本地插件状态更新为 NOT_PUBLISHED;如果获取失败,则保持其原有的 INSTALLED 状态。

    online_plugins_dict = {}
    online_fetched = False
    try:
        with httpx.Client() as client:
            resp = client.get("https://api.soulter.top/astrbot/plugins")
            resp.raise_for_status()
            data = resp.json()
            for plugin_id, plugin_info in data.items():
                online_plugins_dict[str(plugin_id)] = {
                    "name": str(plugin_id),
                    "desc": str(plugin_info.get("desc", "")),
                    "version": str(plugin_info.get("version", "")),
                    "author": str(plugin_info.get("author", "")),
                    "repo": str(plugin_info.get("repo", "")),
                    "status": PluginStatus.NOT_INSTALLED,
                    "local_path": None,
                }
            online_fetched = True
    except Exception as e:
        click.echo(f"Failed to get online plugin list: {e}", err=True)

    # Compare with online plugins and update status
    for local_plugin in result:
        online_plugin = online_plugins_dict.pop(local_plugin["name"], None)
        if online_plugin is None:
            if online_fetched:
                # Local plugin is not published online
                local_plugin["status"] = PluginStatus.NOT_PUBLISHED
            continue

        if (
            VersionComparator.compare_version(
                local_plugin["version"],
                online_plugin["version"],
            )
            < 0
        ):
            local_plugin["status"] = PluginStatus.NEED_UPDATE

    # Add uninstalled online plugins
    result.extend(online_plugins_dict.values())


return result


def manage_plugin(
plugin: dict,
plugins_dir: Path,
Expand Down
111 changes: 111 additions & 0 deletions tests/unit/test_cli_plugin_utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
from pathlib import Path

from astrbot.cli.utils.plugin import PluginStatus, build_plug_list


class FakeResponse:
def raise_for_status(self):
return None

def json(self):
return {
"local-plugin": {
"desc": "remote description",
"version": "2.0.0",
"author": "remote-author",
"repo": "https://example.com/local-plugin",
},
"remote-only": {
"desc": "remote only",
"version": "1.0.0",
"author": "remote-author",
"repo": "https://example.com/remote-only",
},
}


class FakeClient:
def __enter__(self):
return self

def __exit__(self, exc_type, exc, tb):
return False

def get(self, url):
assert url == "https://api.soulter.top/astrbot/plugins"
return FakeResponse()


def write_metadata(plugin_dir: Path, name: str, version: str) -> None:
plugin_dir.mkdir(parents=True, exist_ok=True)
plugin_dir.joinpath("metadata.yaml").write_text(
f"""
name: {name}
desc: local description
version: {version}
author: local-author
repo: https://example.com/{name}
""".strip(),
encoding="utf-8",
)


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}

Comment thread
SeRazon marked this conversation as resolved.
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_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)

Comment thread
SeRazon marked this conversation as resolved.
Comment on lines +69 to +78

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Avoid relying on plugin order in assertions to make the test less brittle.

Both this test and test_build_plug_list_non_existent_path assert the exact list ['local-plugin', 'remote-only']. Even though the current implementation and FakeResponse make this order deterministic, it would be better if the tests didn’t depend on ordering. For example, you could compare the set of names or sort the names before asserting.

Suggested implementation:

    assert {plugin["name"] for plugin in plugins} == {"local-plugin", "remote-only"}
    assert {plugin["name"] for plugin in plugins} == {"local-plugin", "remote-only"}

assert [plugin["name"] for plugin in plugins] == ["local-plugin", "remote-only"]
assert all(plugin["status"] == PluginStatus.NOT_INSTALLED for plugin in plugins)


def test_build_plug_list_local_version_equal_or_newer(monkeypatch, tmp_path):
monkeypatch.setattr("astrbot.cli.utils.plugin.httpx.Client", FakeClient)

# 1. test if local version == remote version
dir_equal = tmp_path / "dir_equal"
write_metadata(dir_equal / "local-plugin", "local-plugin", "2.0.0")

plugins_equal = build_plug_list(dir_equal)
plugins_equal_by_name = {p["name"]: p for p in plugins_equal}
assert plugins_equal_by_name["local-plugin"]["status"] == PluginStatus.INSTALLED

# 2. test if local version > remote version
dir_newer = tmp_path / "dir_newer"
write_metadata(dir_newer / "local-plugin", "local-plugin", "3.0.0")

plugins_newer = build_plug_list(dir_newer)
plugins_newer_by_name = {p["name"]: p for p in plugins_newer}
assert plugins_newer_by_name["local-plugin"]["status"] == PluginStatus.INSTALLED
Comment on lines +83 to +100

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Add coverage for remote-only plugins in the equal/newer version scenarios.

The current test only verifies the local-plugin status. Please also assert that the remote-only plugin is present and remains NOT_INSTALLED in both dir_equal and dir_newer to ensure merge behavior for remote-only plugins is covered when local versions change.

Suggested change
def test_build_plug_list_local_version_equal_or_newer(monkeypatch, tmp_path):
monkeypatch.setattr("astrbot.cli.utils.plugin.httpx.Client", FakeClient)
# 1. test if local version == remote version
dir_equal = tmp_path / "dir_equal"
write_metadata(dir_equal / "local-plugin", "local-plugin", "2.0.0")
plugins_equal = build_plug_list(dir_equal)
plugins_equal_by_name = {p["name"]: p for p in plugins_equal}
assert plugins_equal_by_name["local-plugin"]["status"] == PluginStatus.INSTALLED
# 2. test if local version > remote version
dir_newer = tmp_path / "dir_newer"
write_metadata(dir_newer / "local-plugin", "local-plugin", "3.0.0")
plugins_newer = build_plug_list(dir_newer)
plugins_newer_by_name = {p["name"]: p for p in plugins_newer}
assert plugins_newer_by_name["local-plugin"]["status"] == PluginStatus.INSTALLED
def test_build_plug_list_local_version_equal_or_newer(monkeypatch, tmp_path):
monkeypatch.setattr("astrbot.cli.utils.plugin.httpx.Client", FakeClient)
# 1. test if local version == remote version
dir_equal = tmp_path / "dir_equal"
write_metadata(dir_equal / "local-plugin", "local-plugin", "2.0.0")
plugins_equal = build_plug_list(dir_equal)
plugins_equal_by_name = {p["name"]: p for p in plugins_equal}
# local plugin should be treated as installed when versions are equal
assert plugins_equal_by_name["local-plugin"]["status"] == PluginStatus.INSTALLED
# remote-only plugin should still be present and not installed
assert "remote-only" in plugins_equal_by_name
assert plugins_equal_by_name["remote-only"]["status"] == PluginStatus.NOT_INSTALLED
# 2. test if local version > remote version
dir_newer = tmp_path / "dir_newer"
write_metadata(dir_newer / "local-plugin", "local-plugin", "3.0.0")
plugins_newer = build_plug_list(dir_newer)
plugins_newer_by_name = {p["name"]: p for p in plugins_newer}
# local plugin should be treated as installed when local version is newer
assert plugins_newer_by_name["local-plugin"]["status"] == PluginStatus.INSTALLED
# remote-only plugin should still be present and not installed
assert "remote-only" in plugins_newer_by_name
assert plugins_newer_by_name["remote-only"]["status"] == PluginStatus.NOT_INSTALLED



def test_build_plug_list_non_existent_path(monkeypatch, tmp_path):
non_existent_dir = tmp_path / "completely_non_existent_path"

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

plugins = build_plug_list(non_existent_dir)

assert [plugin["name"] for plugin in plugins] == ["local-plugin", "remote-only"]
assert all(plugin["status"] == PluginStatus.NOT_INSTALLED for plugin in plugins)
Loading