From 6951c1d2fd305499a6954756179be6e2911def70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=82=B9=E6=B0=B8=E8=B5=AB?= <1259085392@qq.com> Date: Fri, 24 Apr 2026 10:57:50 +0900 Subject: [PATCH 01/15] add plugin smoke validation workflow --- .github/workflows/validate-plugin-smoke.yml | 125 ++++ scripts/validate_plugins/__init__.py | 0 scripts/validate_plugins/run.py | 594 ++++++++++++++++++++ tests/__init__.py | 0 tests/test_validate_plugins.py | 143 +++++ 5 files changed, 862 insertions(+) create mode 100644 .github/workflows/validate-plugin-smoke.yml create mode 100644 scripts/validate_plugins/__init__.py create mode 100644 scripts/validate_plugins/run.py create mode 100644 tests/__init__.py create mode 100644 tests/test_validate_plugins.py diff --git a/.github/workflows/validate-plugin-smoke.yml b/.github/workflows/validate-plugin-smoke.yml new file mode 100644 index 00000000..2b5e19bd --- /dev/null +++ b/.github/workflows/validate-plugin-smoke.yml @@ -0,0 +1,125 @@ +name: Validate Plugin Smoke + +on: + pull_request: + paths: + - "plugins.json" + - "scripts/validate_plugins/**" + - ".github/workflows/validate-plugin-smoke.yml" + workflow_dispatch: + inputs: + plugin_names: + description: "Comma-separated plugin keys from plugins.json" + required: false + default: "" + plugin_limit: + description: "Validate the first N plugins when plugin_names is empty" + required: false + default: "20" + astrbot_ref: + description: "AstrBot git ref to validate against" + required: false + default: "master" + +jobs: + validate-plugin-smoke: + runs-on: ubuntu-latest + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Set manual validation inputs + if: github.event_name == 'workflow_dispatch' + run: | + echo "ASTRBOT_REF=${{ inputs.astrbot_ref }}" >> "$GITHUB_ENV" + echo "PLUGIN_NAME_LIST=${{ inputs.plugin_names }}" >> "$GITHUB_ENV" + echo "PLUGIN_LIMIT=${{ inputs.plugin_limit }}" >> "$GITHUB_ENV" + echo "SHOULD_VALIDATE=true" >> "$GITHUB_ENV" + + - name: Detect changed plugins from pull request + if: github.event_name == 'pull_request' + run: | + python - <<'PY' + import json + import os + import subprocess + from pathlib import Path + + base_ref = os.environ["GITHUB_BASE_REF"] + subprocess.run(["git", "fetch", "origin", base_ref, "--depth", "1"], check=True) + base = json.loads( + subprocess.check_output( + ["git", "show", f"origin/{base_ref}:plugins.json"], + text=True, + ) + ) + head = json.loads(Path("plugins.json").read_text(encoding="utf-8")) + + changed = [ + name + for name, payload in head.items() + if base.get(name) != payload + ] + + with open(os.environ["GITHUB_ENV"], "a", encoding="utf-8") as handle: + handle.write("ASTRBOT_REF=master\n") + handle.write(f"PLUGIN_NAME_LIST={','.join(changed)}\n") + handle.write("PLUGIN_LIMIT=\n") + handle.write(f"SHOULD_VALIDATE={'true' if changed else 'false'}\n") + PY + + - name: Show PR diff selection + if: github.event_name == 'pull_request' + run: | + if [ "$SHOULD_VALIDATE" != "true" ]; then + printf '%s\n' "No plugin entries changed in plugins.json; skipping smoke validation." + else + printf 'Selected plugins: %s\n' "$PLUGIN_NAME_LIST" + fi + + - name: Set up Python + if: env.SHOULD_VALIDATE == 'true' + uses: actions/setup-python@v5 + with: + python-version: "3.12" + + - name: Install validator dependencies + if: env.SHOULD_VALIDATE == 'true' + run: python -m pip install --upgrade pip pyyaml + + - name: Clone AstrBot + if: env.SHOULD_VALIDATE == 'true' + run: git clone --depth 1 --branch "$ASTRBOT_REF" "https://github.com/AstrBotDevs/AstrBot" ".cache/AstrBot" + + - name: Install AstrBot dependencies + if: env.SHOULD_VALIDATE == 'true' + run: python -m pip install -r ".cache/AstrBot/requirements.txt" + + - name: Run plugin smoke validator + if: env.SHOULD_VALIDATE == 'true' + run: | + args=( + --astrbot-path ".cache/AstrBot" + --report-path "validation-report.json" + ) + + if [ -n "${PLUGIN_NAME_LIST:-}" ]; then + args+=(--plugin-name-list "$PLUGIN_NAME_LIST") + fi + + if [ -n "${PLUGIN_LIMIT:-}" ]; then + args+=(--limit "$PLUGIN_LIMIT") + fi + + python scripts/validate_plugins/run.py "${args[@]}" + + - name: Upload validation report + if: always() + uses: actions/upload-artifact@v4 + with: + name: validation-report + path: validation-report.json + if-no-files-found: warn diff --git a/scripts/validate_plugins/__init__.py b/scripts/validate_plugins/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/scripts/validate_plugins/run.py b/scripts/validate_plugins/run.py new file mode 100644 index 00000000..7db3d647 --- /dev/null +++ b/scripts/validate_plugins/run.py @@ -0,0 +1,594 @@ +#!/usr/bin/env python3 + +from __future__ import annotations + +import argparse +import asyncio +import json +import os +import re +import shutil +import subprocess +import sys +import tempfile +import traceback +from pathlib import Path +from urllib.parse import urlparse + +try: + import yaml +except ImportError: # pragma: no cover - optional in local unit tests + yaml = None + + +REQUIRED_METADATA_FIELDS = ("name", "desc", "version", "author") + + +def build_result( + *, + plugin: str, + repo: str, + normalized_repo_url: str | None, + ok: bool, + stage: str, + message: str, + plugin_dir_name: str | None = None, + details: dict | str | None = None, +) -> dict: + result = { + "plugin": plugin, + "repo": repo, + "normalized_repo_url": normalized_repo_url, + "ok": ok, + "stage": stage, + "message": message, + } + if plugin_dir_name: + result["plugin_dir_name"] = plugin_dir_name + if details is not None: + result["details"] = details + return result + + +def normalize_repo_url(repo_url: str) -> str: + parsed = urlparse(repo_url.strip()) + if parsed.scheme not in {"http", "https"}: + raise ValueError("repo URL must use http or https") + if parsed.netloc.lower() != "github.com": + raise ValueError("repo URL must point to github.com") + + parts = [part for part in parsed.path.split("/") if part] + if len(parts) != 2: + raise ValueError("repo URL must include owner and repository") + + owner, repo = parts[0], parts[1] + if repo.endswith(".git"): + repo = repo[:-4] + if not owner or not repo: + raise ValueError("repo URL owner or repository is empty") + + return f"https://github.com/{owner}/{repo}" + + +def select_plugins( + *, + plugins: dict, + requested_names: list[str] | None, + limit: int | None, +) -> list[tuple[str, dict]]: + if requested_names: + selected = [] + for name in requested_names: + if name not in plugins: + raise KeyError(f"plugin not found: {name}") + selected.append((name, plugins[name])) + return selected + + items = list(plugins.items()) + if limit is None: + return items + return items[:limit] + + +def _parse_simple_yaml(path: Path) -> dict: + result = {} + for raw_line in path.read_text(encoding="utf-8").splitlines(): + line = raw_line.strip() + if not line or line.startswith("#") or ":" not in line: + continue + key, value = line.split(":", 1) + result[key.strip()] = value.strip().strip("\"'") + return result + + +def load_metadata(path: Path) -> dict: + if yaml is not None: + loaded = yaml.safe_load(path.read_text(encoding="utf-8")) + if isinstance(loaded, dict): + return loaded + return {} + return _parse_simple_yaml(path) + + +def precheck_plugin_directory(plugin_dir: Path) -> dict: + metadata_path = plugin_dir / "metadata.yaml" + if not metadata_path.exists(): + return { + "ok": False, + "stage": "metadata", + "message": "missing metadata.yaml", + } + + metadata = load_metadata(metadata_path) + missing = [ + field + for field in REQUIRED_METADATA_FIELDS + if not isinstance(metadata.get(field), str) or not metadata[field].strip() + ] + if missing: + return { + "ok": False, + "stage": "metadata", + "message": f"missing required metadata fields: {', '.join(missing)}", + } + + plugin_name = metadata["name"].strip() + entry_candidates = [plugin_dir / "main.py", plugin_dir / f"{plugin_name}.py"] + if not any(path.exists() for path in entry_candidates): + return { + "ok": False, + "stage": "entrypoint", + "message": f"missing main.py or {plugin_name}.py", + } + + return { + "ok": True, + "stage": "precheck", + "message": "ok", + "metadata": metadata, + "plugin_dir_name": plugin_name, + } + + +def build_worker_command( + *, + script_path: Path, + astrbot_path: Path, + plugin_source_dir: Path, + plugin_dir_name: str, + normalized_repo_url: str, +) -> list[str]: + return [ + sys.executable, + str(script_path), + "--worker", + "--astrbot-path", + str(astrbot_path), + "--plugin-source-dir", + str(plugin_source_dir), + "--plugin-dir-name", + plugin_dir_name, + "--normalized-repo-url", + normalized_repo_url, + ] + + +def build_report(results: list[dict]) -> dict: + passed = sum(1 for result in results if result.get("ok")) + failed = len(results) - passed + return { + "summary": { + "total": len(results), + "passed": passed, + "failed": failed, + }, + "results": results, + } + + +def load_plugins_index(path: Path) -> dict[str, dict]: + data = json.loads(path.read_text(encoding="utf-8")) + if not isinstance(data, dict): + raise ValueError("plugins.json must contain a JSON object") + result = {} + for key, value in data.items(): + if isinstance(key, str) and isinstance(value, dict): + result[key] = value + return result + + +def combine_requested_names( + plugin_names: list[str] | None, + plugin_name_list: str | None, +) -> list[str]: + names = list(plugin_names or []) + if plugin_name_list: + names.extend(part.strip() for part in plugin_name_list.split(",")) + return [name for name in names if name] + + +def sanitize_name(name: str) -> str: + sanitized = re.sub(r"[^A-Za-z0-9._-]+", "-", name).strip("-") + return sanitized or "plugin" + + +def clone_plugin_repo(repo_url: str, destination: Path) -> None: + subprocess.run( + ["git", "clone", "--depth", "1", repo_url, str(destination)], + check=True, + capture_output=True, + text=True, + ) + + +def parse_worker_output( + *, + plugin: str, + repo: str, + normalized_repo_url: str, + completed: subprocess.CompletedProcess[str], + plugin_dir_name: str, +) -> dict: + stdout = completed.stdout.strip() + if stdout: + try: + payload = json.loads(stdout) + except json.JSONDecodeError: + payload = None + if isinstance(payload, dict): + payload["plugin"] = plugin + payload["repo"] = repo + payload["normalized_repo_url"] = normalized_repo_url + payload.setdefault("plugin_dir_name", plugin_dir_name) + return payload + + stderr = completed.stderr.strip() + message = stderr or stdout or "worker returned no structured output" + return build_result( + plugin=plugin, + repo=repo, + normalized_repo_url=normalized_repo_url, + ok=False, + stage="worker", + message=message, + plugin_dir_name=plugin_dir_name, + ) + + +def validate_plugin( + *, + plugin: str, + plugin_data: dict, + astrbot_path: Path, + script_path: Path, + work_dir: Path, + load_timeout: int, +) -> dict: + repo_url = plugin_data.get("repo") + if not isinstance(repo_url, str) or not repo_url.strip(): + return build_result( + plugin=plugin, + repo="", + normalized_repo_url=None, + ok=False, + stage="repo_url", + message="missing repo field", + ) + + try: + normalized_repo_url = normalize_repo_url(repo_url) + except ValueError as exc: + return build_result( + plugin=plugin, + repo=repo_url, + normalized_repo_url=None, + ok=False, + stage="repo_url", + message=str(exc), + ) + + plugin_clone_dir = work_dir / sanitize_name(plugin) + try: + clone_plugin_repo(normalized_repo_url, plugin_clone_dir) + except subprocess.CalledProcessError as exc: + message = exc.stderr.strip() or exc.stdout.strip() or str(exc) + return build_result( + plugin=plugin, + repo=repo_url, + normalized_repo_url=normalized_repo_url, + ok=False, + stage="clone", + message=message, + ) + + precheck = precheck_plugin_directory(plugin_clone_dir) + if not precheck["ok"]: + return build_result( + plugin=plugin, + repo=repo_url, + normalized_repo_url=normalized_repo_url, + ok=False, + stage=precheck["stage"], + message=precheck["message"], + ) + + plugin_dir_name = precheck["plugin_dir_name"] + command = build_worker_command( + script_path=script_path, + astrbot_path=astrbot_path, + plugin_source_dir=plugin_clone_dir, + plugin_dir_name=plugin_dir_name, + normalized_repo_url=normalized_repo_url, + ) + + try: + completed = subprocess.run( + command, + check=False, + capture_output=True, + text=True, + timeout=load_timeout, + ) + except subprocess.TimeoutExpired: + return build_result( + plugin=plugin, + repo=repo_url, + normalized_repo_url=normalized_repo_url, + ok=False, + stage="timeout", + message=f"worker timed out after {load_timeout} seconds", + plugin_dir_name=plugin_dir_name, + ) + + return parse_worker_output( + plugin=plugin, + repo=repo_url, + normalized_repo_url=normalized_repo_url, + completed=completed, + plugin_dir_name=plugin_dir_name, + ) + + +class NullStub: + def __getattr__(self, name: str) -> "NullStub": + del name + return self + + def __call__(self, *args, **kwargs) -> "NullStub": + del args, kwargs + return self + + def __await__(self): + async def _return_self(): + return self + + return _return_self().__await__() + + def __iter__(self): + return iter(()) + + def __bool__(self) -> bool: + return False + + +class DummyContext: + def __init__(self) -> None: + self._star_manager = None + + def get_all_stars(self): + try: + from astrbot.core.star.star import star_registry + + return list(star_registry) + except Exception: + return [] + + def get_registered_star(self, star_name: str): + for star in self.get_all_stars(): + if getattr(star, "name", None) == star_name: + return star + return None + + def activate_llm_tool(self, name: str) -> bool: + del name + return True + + def deactivate_llm_tool(self, name: str) -> bool: + del name + return True + + def register_llm_tool(self, name: str, func_args, desc: str, func_obj) -> None: + del name, func_args, desc, func_obj + + def unregister_llm_tool(self, name: str) -> None: + del name + + def __getattr__(self, name: str) -> NullStub: + del name + return NullStub() + + +async def run_worker_load_check(plugin_dir_name: str, normalized_repo_url: str) -> dict: + try: + from astrbot.core.star.star_manager import PluginManager + except Exception as exc: + return build_result( + plugin=plugin_dir_name, + repo=normalized_repo_url, + normalized_repo_url=normalized_repo_url, + ok=False, + stage="astrbot_import", + message=str(exc), + plugin_dir_name=plugin_dir_name, + details=traceback.format_exc(), + ) + + context = DummyContext() + manager = PluginManager(context, {}) + + try: + success, error = await manager.load(specified_dir_name=plugin_dir_name) + except Exception as exc: + return build_result( + plugin=plugin_dir_name, + repo=normalized_repo_url, + normalized_repo_url=normalized_repo_url, + ok=False, + stage="load", + message=str(exc), + plugin_dir_name=plugin_dir_name, + details=traceback.format_exc(), + ) + + if success: + return build_result( + plugin=plugin_dir_name, + repo=normalized_repo_url, + normalized_repo_url=normalized_repo_url, + ok=True, + stage="load", + message="plugin loaded successfully", + plugin_dir_name=plugin_dir_name, + ) + + return build_result( + plugin=plugin_dir_name, + repo=normalized_repo_url, + normalized_repo_url=normalized_repo_url, + ok=False, + stage="load", + message=error or "plugin load failed", + plugin_dir_name=plugin_dir_name, + details=manager.failed_plugin_dict.get(plugin_dir_name), + ) + + +def run_worker(args: argparse.Namespace) -> int: + temp_root = Path(tempfile.mkdtemp(prefix="astrbot-plugin-worker-")) + try: + astrbot_root = temp_root / "astrbot-root" + plugin_store = astrbot_root / "data" / "plugins" + plugin_config = astrbot_root / "data" / "config" + plugin_store.mkdir(parents=True, exist_ok=True) + plugin_config.mkdir(parents=True, exist_ok=True) + + source_dir = Path(args.plugin_source_dir).resolve() + target_dir = plugin_store / args.plugin_dir_name + shutil.copytree(source_dir, target_dir, dirs_exist_ok=True) + + os.environ["ASTRBOT_ROOT"] = str(astrbot_root) + os.environ.setdefault("TESTING", "true") + sys.path.insert(0, str(Path(args.astrbot_path).resolve())) + + result = asyncio.run( + run_worker_load_check(args.plugin_dir_name, args.normalized_repo_url) + ) + except Exception as exc: + result = build_result( + plugin=args.plugin_dir_name, + repo=args.normalized_repo_url, + normalized_repo_url=args.normalized_repo_url, + ok=False, + stage="worker", + message=str(exc), + plugin_dir_name=args.plugin_dir_name, + details=traceback.format_exc(), + ) + finally: + shutil.rmtree(temp_root, ignore_errors=True) + + print(json.dumps(result, ensure_ascii=False)) + return 0 if result["ok"] else 1 + + +def build_parser() -> argparse.ArgumentParser: + parser = argparse.ArgumentParser(description="Validate AstrBot plugins") + parser.add_argument("--plugins-json", default="plugins.json") + parser.add_argument("--plugin-name", action="append", dest="plugin_names") + parser.add_argument("--plugin-name-list") + parser.add_argument("--limit", type=int) + parser.add_argument("--astrbot-path") + parser.add_argument("--report-path", default="validation-report.json") + parser.add_argument("--work-dir") + parser.add_argument("--load-timeout", type=int, default=300) + parser.add_argument("--worker", action="store_true") + parser.add_argument("--plugin-source-dir") + parser.add_argument("--plugin-dir-name") + parser.add_argument("--normalized-repo-url") + return parser + + +def main() -> int: + parser = build_parser() + args = parser.parse_args() + + if args.worker: + missing = [ + flag + for flag, value in ( + ("--astrbot-path", args.astrbot_path), + ("--plugin-source-dir", args.plugin_source_dir), + ("--plugin-dir-name", args.plugin_dir_name), + ("--normalized-repo-url", args.normalized_repo_url), + ) + if not value + ] + if missing: + parser.error(f"worker mode requires: {', '.join(missing)}") + return run_worker(args) + + if not args.astrbot_path: + parser.error("--astrbot-path is required") + + requested_names = combine_requested_names(args.plugin_names, args.plugin_name_list) + plugins = load_plugins_index(Path(args.plugins_json)) + selected = select_plugins( + plugins=plugins, + requested_names=requested_names or None, + limit=args.limit, + ) + + temp_dir = None + work_dir = Path(args.work_dir) if args.work_dir else None + if work_dir is None: + temp_dir = tempfile.TemporaryDirectory(prefix="astrbot-plugin-validate-") + work_dir = Path(temp_dir.name) + work_dir.mkdir(parents=True, exist_ok=True) + + try: + results = [ + validate_plugin( + plugin=plugin, + plugin_data=plugin_data, + astrbot_path=Path(args.astrbot_path).resolve(), + script_path=Path(__file__).resolve(), + work_dir=work_dir, + load_timeout=args.load_timeout, + ) + for plugin, plugin_data in selected + ] + finally: + if temp_dir is not None: + temp_dir.cleanup() + + report = build_report(results) + report_path = Path(args.report_path) + report_path.write_text( + json.dumps(report, ensure_ascii=False, indent=2) + "\n", + encoding="utf-8", + ) + + print( + json.dumps( + { + "report_path": str(report_path), + "summary": report["summary"], + }, + ensure_ascii=False, + ) + ) + return 0 if report["summary"]["failed"] == 0 else 1 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/test_validate_plugins.py b/tests/test_validate_plugins.py new file mode 100644 index 00000000..a561767f --- /dev/null +++ b/tests/test_validate_plugins.py @@ -0,0 +1,143 @@ +import importlib.util +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[1] +MODULE_PATH = ROOT / "scripts" / "validate_plugins" / "run.py" + + +def load_validator_module(): + if not MODULE_PATH.exists(): + raise AssertionError(f"validator script missing: {MODULE_PATH}") + + spec = importlib.util.spec_from_file_location("validate_plugins_run", MODULE_PATH) + if spec is None or spec.loader is None: + raise AssertionError("unable to load validator module spec") + + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +class NormalizeRepoUrlTests(unittest.TestCase): + def test_strips_git_suffix_trailing_slash_and_query(self): + module = load_validator_module() + + self.assertEqual( + module.normalize_repo_url( + "https://github.com/example/demo-plugin.git/?tab=readme-ov-file" + ), + "https://github.com/example/demo-plugin", + ) + + def test_rejects_non_github_urls(self): + module = load_validator_module() + + with self.assertRaises(ValueError): + module.normalize_repo_url("https://gitlab.com/example/demo-plugin") + + +class SelectPluginsTests(unittest.TestCase): + def test_prefers_explicit_names_in_requested_order(self): + module = load_validator_module() + plugins = { + "plugin-a": {"repo": "https://github.com/example/plugin-a"}, + "plugin-b": {"repo": "https://github.com/example/plugin-b"}, + "plugin-c": {"repo": "https://github.com/example/plugin-c"}, + } + + selected = module.select_plugins( + plugins=plugins, + requested_names=["plugin-c", "plugin-a"], + limit=None, + ) + + self.assertEqual([item[0] for item in selected], ["plugin-c", "plugin-a"]) + + +class MetadataValidationTests(unittest.TestCase): + def test_reports_missing_required_metadata_fields(self): + module = load_validator_module() + + with tempfile.TemporaryDirectory() as tmp_dir: + plugin_dir = Path(tmp_dir) + (plugin_dir / "metadata.yaml").write_text( + "name: demo_plugin\nauthor: AstrBot Team\n", + encoding="utf-8", + ) + (plugin_dir / "main.py").write_text("print('hello')\n", encoding="utf-8") + + result = module.precheck_plugin_directory(plugin_dir) + + self.assertFalse(result["ok"]) + self.assertEqual(result["stage"], "metadata") + self.assertIn("desc", result["message"]) + self.assertIn("version", result["message"]) + + +class WorkerCommandTests(unittest.TestCase): + def test_build_worker_command_contains_required_arguments(self): + module = load_validator_module() + + command = module.build_worker_command( + script_path=Path("/tmp/run.py"), + astrbot_path=Path("/tmp/astrbot"), + plugin_source_dir=Path("/tmp/plugin-src"), + plugin_dir_name="demo_plugin", + normalized_repo_url="https://github.com/example/demo-plugin", + ) + + self.assertEqual(command[0], sys.executable) + self.assertEqual(command[1], "/tmp/run.py") + self.assertIn("--worker", command) + self.assertIn("--astrbot-path", command) + self.assertIn("--plugin-source-dir", command) + self.assertIn("--plugin-dir-name", command) + self.assertIn("--normalized-repo-url", command) + + +class ReportBuilderTests(unittest.TestCase): + def test_build_report_counts_passed_and_failed_results(self): + module = load_validator_module() + + report = module.build_report( + [ + {"plugin": "plugin-a", "ok": True, "stage": "load", "message": "ok"}, + {"plugin": "plugin-b", "ok": False, "stage": "metadata", "message": "missing desc"}, + ] + ) + + self.assertEqual(report["summary"]["total"], 2) + self.assertEqual(report["summary"]["passed"], 1) + self.assertEqual(report["summary"]["failed"], 1) + self.assertEqual(report["results"][1]["plugin"], "plugin-b") + + +class WorkerOutputParsingTests(unittest.TestCase): + def test_parse_worker_output_keeps_market_plugin_key(self): + module = load_validator_module() + completed = subprocess.CompletedProcess( + args=["python3", "run.py"], + returncode=1, + stdout='{"plugin": "demo_plugin", "ok": false, "stage": "load", "message": "boom"}', + stderr="", + ) + + result = module.parse_worker_output( + plugin="market-plugin-key", + repo="https://github.com/example/demo-plugin?tab=readme-ov-file", + normalized_repo_url="https://github.com/example/demo-plugin", + completed=completed, + plugin_dir_name="demo_plugin", + ) + + self.assertEqual(result["plugin"], "market-plugin-key") + self.assertEqual(result["plugin_dir_name"], "demo_plugin") + + +if __name__ == "__main__": + unittest.main() From 4f18133c1b12f6fa2ceb4d5860db7dfc8d745c10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=82=B9=E6=B0=B8=E8=B5=AB?= <1259085392@qq.com> Date: Fri, 24 Apr 2026 11:20:01 +0900 Subject: [PATCH 02/15] fix plugin smoke worker import path --- .github/workflows/validate-plugin-smoke.yml | 4 +- scripts/validate_plugins/run.py | 41 +++++++++---- tests/test_validate_plugins.py | 66 +++++++++++++++++++++ 3 files changed, 96 insertions(+), 15 deletions(-) diff --git a/.github/workflows/validate-plugin-smoke.yml b/.github/workflows/validate-plugin-smoke.yml index 2b5e19bd..a266d447 100644 --- a/.github/workflows/validate-plugin-smoke.yml +++ b/.github/workflows/validate-plugin-smoke.yml @@ -13,9 +13,9 @@ on: required: false default: "" plugin_limit: - description: "Validate the first N plugins when plugin_names is empty" + description: "Validate the first N plugins when plugin_names is empty. Leave blank or use -1 for all plugins" required: false - default: "20" + default: "" astrbot_ref: description: "AstrBot git ref to validate against" required: false diff --git a/scripts/validate_plugins/run.py b/scripts/validate_plugins/run.py index 7db3d647..582413fd 100644 --- a/scripts/validate_plugins/run.py +++ b/scripts/validate_plugins/run.py @@ -85,7 +85,7 @@ def select_plugins( return selected items = list(plugins.items()) - if limit is None: + if limit is None or limit < 0: return items return items[:limit] @@ -173,6 +173,10 @@ def build_worker_command( ] +def build_worker_sys_path(*, astrbot_root: Path, astrbot_path: Path) -> list[str]: + return [str(astrbot_root.resolve()), str(astrbot_path.resolve())] + + def build_report(results: list[dict]) -> dict: passed = sum(1 for result in results if result.get("ok")) failed = len(results) - passed @@ -231,16 +235,17 @@ def parse_worker_output( ) -> dict: stdout = completed.stdout.strip() if stdout: - try: - payload = json.loads(stdout) - except json.JSONDecodeError: - payload = None - if isinstance(payload, dict): - payload["plugin"] = plugin - payload["repo"] = repo - payload["normalized_repo_url"] = normalized_repo_url - payload.setdefault("plugin_dir_name", plugin_dir_name) - return payload + for line in reversed(stdout.splitlines()): + try: + payload = json.loads(line) + except json.JSONDecodeError: + continue + if isinstance(payload, dict): + payload["plugin"] = plugin + payload["repo"] = repo + payload["normalized_repo_url"] = normalized_repo_url + payload.setdefault("plugin_dir_name", plugin_dir_name) + return payload stderr = completed.stderr.strip() message = stderr or stdout or "worker returned no structured output" @@ -478,7 +483,13 @@ def run_worker(args: argparse.Namespace) -> int: os.environ["ASTRBOT_ROOT"] = str(astrbot_root) os.environ.setdefault("TESTING", "true") - sys.path.insert(0, str(Path(args.astrbot_path).resolve())) + for entry in reversed( + build_worker_sys_path( + astrbot_root=astrbot_root, + astrbot_path=Path(args.astrbot_path), + ) + ): + sys.path.insert(0, entry) result = asyncio.run( run_worker_load_check(args.plugin_dir_name, args.normalized_repo_url) @@ -506,7 +517,11 @@ def build_parser() -> argparse.ArgumentParser: parser.add_argument("--plugins-json", default="plugins.json") parser.add_argument("--plugin-name", action="append", dest="plugin_names") parser.add_argument("--plugin-name-list") - parser.add_argument("--limit", type=int) + parser.add_argument( + "--limit", + type=int, + help="Validate the first N plugins when plugin names are empty. Omit or use -1 for all plugins.", + ) parser.add_argument("--astrbot-path") parser.add_argument("--report-path", default="validation-report.json") parser.add_argument("--work-dir") diff --git a/tests/test_validate_plugins.py b/tests/test_validate_plugins.py index a561767f..ab086052 100644 --- a/tests/test_validate_plugins.py +++ b/tests/test_validate_plugins.py @@ -42,6 +42,36 @@ def test_rejects_non_github_urls(self): class SelectPluginsTests(unittest.TestCase): + def test_returns_all_plugins_when_limit_is_none(self): + module = load_validator_module() + plugins = { + "plugin-a": {"repo": "https://github.com/example/plugin-a"}, + "plugin-b": {"repo": "https://github.com/example/plugin-b"}, + } + + selected = module.select_plugins( + plugins=plugins, + requested_names=None, + limit=None, + ) + + self.assertEqual([item[0] for item in selected], ["plugin-a", "plugin-b"]) + + def test_returns_all_plugins_when_limit_is_negative_one(self): + module = load_validator_module() + plugins = { + "plugin-a": {"repo": "https://github.com/example/plugin-a"}, + "plugin-b": {"repo": "https://github.com/example/plugin-b"}, + } + + selected = module.select_plugins( + plugins=plugins, + requested_names=None, + limit=-1, + ) + + self.assertEqual([item[0] for item in selected], ["plugin-a", "plugin-b"]) + def test_prefers_explicit_names_in_requested_order(self): module = load_validator_module() plugins = { @@ -100,6 +130,21 @@ def test_build_worker_command_contains_required_arguments(self): self.assertIn("--normalized-repo-url", command) +class WorkerSysPathTests(unittest.TestCase): + def test_worker_sys_path_includes_astrbot_root_before_codebase(self): + module = load_validator_module() + + sys_path_entries = module.build_worker_sys_path( + astrbot_root=Path("/tmp/astrbot-root"), + astrbot_path=Path("/tmp/AstrBot"), + ) + + self.assertEqual( + [Path(item) for item in sys_path_entries], + [Path("/tmp/astrbot-root").resolve(), Path("/tmp/AstrBot").resolve()], + ) + + class ReportBuilderTests(unittest.TestCase): def test_build_report_counts_passed_and_failed_results(self): module = load_validator_module() @@ -138,6 +183,27 @@ def test_parse_worker_output_keeps_market_plugin_key(self): self.assertEqual(result["plugin"], "market-plugin-key") self.assertEqual(result["plugin_dir_name"], "demo_plugin") + def test_parse_worker_output_uses_last_json_line_after_logs(self): + module = load_validator_module() + completed = subprocess.CompletedProcess( + args=["python3", "run.py"], + returncode=1, + stdout='log line\n{"plugin": "demo_plugin", "ok": false, "stage": "load", "message": "boom"}', + stderr="", + ) + + result = module.parse_worker_output( + plugin="market-plugin-key", + repo="https://github.com/example/demo-plugin", + normalized_repo_url="https://github.com/example/demo-plugin", + completed=completed, + plugin_dir_name="demo_plugin", + ) + + self.assertEqual(result["plugin"], "market-plugin-key") + self.assertEqual(result["stage"], "load") + self.assertEqual(result["message"], "boom") + if __name__ == "__main__": unittest.main() From 780db1a290a7189a4f717856248265f4d26f59d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=82=B9=E6=B0=B8=E8=B5=AB?= <1259085392@qq.com> Date: Fri, 24 Apr 2026 11:39:14 +0900 Subject: [PATCH 03/15] harden plugin smoke validation edge cases --- .github/workflows/validate-plugin-smoke.yml | 44 +++++--- scripts/validate_plugins/run.py | 76 +++++++++++--- tests/test_validate_plugins.py | 107 ++++++++++++++++++++ 3 files changed, 203 insertions(+), 24 deletions(-) diff --git a/.github/workflows/validate-plugin-smoke.yml b/.github/workflows/validate-plugin-smoke.yml index a266d447..bb9d1470 100644 --- a/.github/workflows/validate-plugin-smoke.yml +++ b/.github/workflows/validate-plugin-smoke.yml @@ -50,32 +50,52 @@ jobs: base_ref = os.environ["GITHUB_BASE_REF"] subprocess.run(["git", "fetch", "origin", base_ref, "--depth", "1"], check=True) - base = json.loads( - subprocess.check_output( + + validation_note = "" + + try: + base_raw = subprocess.check_output( ["git", "show", f"origin/{base_ref}:plugins.json"], text=True, + stderr=subprocess.DEVNULL, ) - ) - head = json.loads(Path("plugins.json").read_text(encoding="utf-8")) - - changed = [ - name - for name, payload in head.items() - if base.get(name) != payload - ] + base = json.loads(base_raw) + if not isinstance(base, dict): + base = {} + except (subprocess.CalledProcessError, json.JSONDecodeError): + base = {} + + try: + head = json.loads(Path("plugins.json").read_text(encoding="utf-8")) + if not isinstance(head, dict): + raise ValueError("plugins.json must contain a JSON object") + except (json.JSONDecodeError, ValueError) as exc: + changed = [] + should_validate = False + validation_note = f"Skipping smoke validation because plugins.json is invalid on the PR head: {exc}" + else: + changed = [ + name + for name, payload in head.items() + if base.get(name) != payload + ] + should_validate = bool(changed) + if not should_validate: + validation_note = "No plugin entries changed in plugins.json; skipping smoke validation." with open(os.environ["GITHUB_ENV"], "a", encoding="utf-8") as handle: handle.write("ASTRBOT_REF=master\n") handle.write(f"PLUGIN_NAME_LIST={','.join(changed)}\n") handle.write("PLUGIN_LIMIT=\n") - handle.write(f"SHOULD_VALIDATE={'true' if changed else 'false'}\n") + handle.write(f"SHOULD_VALIDATE={'true' if should_validate else 'false'}\n") + handle.write(f"VALIDATION_NOTE={validation_note}\n") PY - name: Show PR diff selection if: github.event_name == 'pull_request' run: | if [ "$SHOULD_VALIDATE" != "true" ]; then - printf '%s\n' "No plugin entries changed in plugins.json; skipping smoke validation." + printf '%s\n' "${VALIDATION_NOTE:-Smoke validation skipped.}" else printf 'Selected plugins: %s\n' "$PLUGIN_NAME_LIST" fi diff --git a/scripts/validate_plugins/run.py b/scripts/validate_plugins/run.py index 582413fd..ebf8ee23 100644 --- a/scripts/validate_plugins/run.py +++ b/scripts/validate_plugins/run.py @@ -4,6 +4,7 @@ import argparse import asyncio +import hashlib import json import os import re @@ -22,6 +23,7 @@ REQUIRED_METADATA_FIELDS = ("name", "desc", "version", "author") +DEFAULT_CLONE_TIMEOUT = 120 def build_result( @@ -205,7 +207,7 @@ def combine_requested_names( plugin_names: list[str] | None, plugin_name_list: str | None, ) -> list[str]: - names = list(plugin_names or []) + names = [name.strip() for name in (plugin_names or [])] if plugin_name_list: names.extend(part.strip() for part in plugin_name_list.split(",")) return [name for name in names if name] @@ -216,12 +218,47 @@ def sanitize_name(name: str) -> str: return sanitized or "plugin" -def clone_plugin_repo(repo_url: str, destination: Path) -> None: +def build_plugin_clone_dir(work_dir: Path, plugin: str) -> Path: + digest = hashlib.sha256(plugin.encode("utf-8")).hexdigest()[:8] + return work_dir / f"{sanitize_name(plugin)}-{digest}" + + +def _normalize_process_output(output: str | bytes | None) -> str | None: + if output is None: + return None + if isinstance(output, bytes): + output = output.decode("utf-8", errors="replace") + normalized = output.strip() + return normalized or None + + +def build_process_output_details( + *, + stdout: str | bytes | None, + stderr: str | bytes | None, +) -> dict | None: + details = {} + stdout_text = _normalize_process_output(stdout) + stderr_text = _normalize_process_output(stderr) + if stdout_text: + details["stdout"] = stdout_text + if stderr_text: + details["stderr"] = stderr_text + return details or None + + +def clone_plugin_repo( + repo_url: str, + destination: Path, + *, + timeout: int = DEFAULT_CLONE_TIMEOUT, +) -> None: subprocess.run( ["git", "clone", "--depth", "1", repo_url, str(destination)], check=True, capture_output=True, text=True, + timeout=timeout, ) @@ -267,6 +304,7 @@ def validate_plugin( astrbot_path: Path, script_path: Path, work_dir: Path, + clone_timeout: int, load_timeout: int, ) -> dict: repo_url = plugin_data.get("repo") @@ -292,9 +330,13 @@ def validate_plugin( message=str(exc), ) - plugin_clone_dir = work_dir / sanitize_name(plugin) + plugin_clone_dir = build_plugin_clone_dir(work_dir, plugin) try: - clone_plugin_repo(normalized_repo_url, plugin_clone_dir) + clone_plugin_repo( + normalized_repo_url, + plugin_clone_dir, + timeout=clone_timeout, + ) except subprocess.CalledProcessError as exc: message = exc.stderr.strip() or exc.stdout.strip() or str(exc) return build_result( @@ -305,6 +347,16 @@ def validate_plugin( stage="clone", message=message, ) + except subprocess.TimeoutExpired as exc: + return build_result( + plugin=plugin, + repo=repo_url, + normalized_repo_url=normalized_repo_url, + ok=False, + stage="clone_timeout", + message=f"git clone timed out after {clone_timeout} seconds", + details=build_process_output_details(stdout=exc.stdout, stderr=exc.stderr), + ) precheck = precheck_plugin_directory(plugin_clone_dir) if not precheck["ok"]: @@ -334,7 +386,7 @@ def validate_plugin( text=True, timeout=load_timeout, ) - except subprocess.TimeoutExpired: + except subprocess.TimeoutExpired as exc: return build_result( plugin=plugin, repo=repo_url, @@ -343,6 +395,7 @@ def validate_plugin( stage="timeout", message=f"worker timed out after {load_timeout} seconds", plugin_dir_name=plugin_dir_name, + details=build_process_output_details(stdout=exc.stdout, stderr=exc.stderr), ) return parse_worker_output( @@ -483,13 +536,10 @@ def run_worker(args: argparse.Namespace) -> int: os.environ["ASTRBOT_ROOT"] = str(astrbot_root) os.environ.setdefault("TESTING", "true") - for entry in reversed( - build_worker_sys_path( - astrbot_root=astrbot_root, - astrbot_path=Path(args.astrbot_path), - ) - ): - sys.path.insert(0, entry) + sys.path[:0] = build_worker_sys_path( + astrbot_root=astrbot_root, + astrbot_path=Path(args.astrbot_path), + ) result = asyncio.run( run_worker_load_check(args.plugin_dir_name, args.normalized_repo_url) @@ -525,6 +575,7 @@ def build_parser() -> argparse.ArgumentParser: parser.add_argument("--astrbot-path") parser.add_argument("--report-path", default="validation-report.json") parser.add_argument("--work-dir") + parser.add_argument("--clone-timeout", type=int, default=DEFAULT_CLONE_TIMEOUT) parser.add_argument("--load-timeout", type=int, default=300) parser.add_argument("--worker", action="store_true") parser.add_argument("--plugin-source-dir") @@ -578,6 +629,7 @@ def main() -> int: astrbot_path=Path(args.astrbot_path).resolve(), script_path=Path(__file__).resolve(), work_dir=work_dir, + clone_timeout=args.clone_timeout, load_timeout=args.load_timeout, ) for plugin, plugin_data in selected diff --git a/tests/test_validate_plugins.py b/tests/test_validate_plugins.py index ab086052..18ab69da 100644 --- a/tests/test_validate_plugins.py +++ b/tests/test_validate_plugins.py @@ -40,6 +40,39 @@ def test_rejects_non_github_urls(self): with self.assertRaises(ValueError): module.normalize_repo_url("https://gitlab.com/example/demo-plugin") + def test_rejects_non_http_schemes(self): + module = load_validator_module() + + for url in ( + "git://github.com/example/demo-plugin", + "ssh://github.com/example/demo-plugin", + ): + with self.subTest(url=url): + with self.assertRaisesRegex(ValueError, "repo URL must use http or https"): + module.normalize_repo_url(url) + + def test_rejects_missing_owner_or_repository(self): + module = load_validator_module() + + for url in ( + "https://github.com/", + "https://github.com/example", + "https://github.com/example/", + "https://github.com//demo-plugin", + "https://github.com/example//", + ): + with self.subTest(url=url): + with self.assertRaisesRegex(ValueError, "repo URL must include owner and repository"): + module.normalize_repo_url(url) + + def test_strips_leading_and_trailing_whitespace(self): + module = load_validator_module() + + self.assertEqual( + module.normalize_repo_url(" https://github.com/example/demo-plugin "), + "https://github.com/example/demo-plugin", + ) + class SelectPluginsTests(unittest.TestCase): def test_returns_all_plugins_when_limit_is_none(self): @@ -88,6 +121,80 @@ def test_prefers_explicit_names_in_requested_order(self): self.assertEqual([item[0] for item in selected], ["plugin-c", "plugin-a"]) + def test_respects_positive_limit_when_names_not_requested(self): + module = load_validator_module() + plugins = { + "plugin-a": {"repo": "https://github.com/example/plugin-a"}, + "plugin-b": {"repo": "https://github.com/example/plugin-b"}, + "plugin-c": {"repo": "https://github.com/example/plugin-c"}, + } + + selected = module.select_plugins( + plugins=plugins, + requested_names=None, + limit=1, + ) + + self.assertEqual([item[0] for item in selected], ["plugin-a"]) + + def test_raises_key_error_for_unknown_requested_plugin(self): + module = load_validator_module() + plugins = { + "known-plugin": {"repo": "https://github.com/example/known-plugin"}, + } + + with self.assertRaisesRegex(KeyError, "plugin not found: missing-plugin"): + module.select_plugins( + plugins=plugins, + requested_names=["known-plugin", "missing-plugin"], + limit=None, + ) + + +class HelperFunctionTests(unittest.TestCase): + def test_combine_requested_names_merges_trims_and_drops_empty_values(self): + module = load_validator_module() + + combined = module.combine_requested_names( + plugin_names=["foo", " bar ", "", " "], + plugin_name_list="baz, qux , ,foo ", + ) + + self.assertEqual(combined, ["foo", "bar", "baz", "qux", "foo"]) + + def test_combine_requested_names_handles_none_inputs(self): + module = load_validator_module() + + self.assertEqual(module.combine_requested_names(None, None), []) + + def test_sanitize_name_replaces_invalid_chars_and_falls_back_when_needed(self): + module = load_validator_module() + + self.assertEqual(module.sanitize_name(" -invalid name!*?- "), "invalid-name") + self.assertEqual(module.sanitize_name("valid-name_123"), "valid-name_123") + self.assertEqual(module.sanitize_name(" "), "plugin") + self.assertEqual(module.sanitize_name("!!!"), "plugin") + + def test_build_plugin_clone_dir_is_unique_for_colliding_sanitized_names(self): + module = load_validator_module() + + first = module.build_plugin_clone_dir(Path("/tmp/work"), "foo bar") + second = module.build_plugin_clone_dir(Path("/tmp/work"), "foo/bar") + + self.assertNotEqual(first, second) + self.assertEqual(first.parent, Path("/tmp/work")) + self.assertEqual(second.parent, Path("/tmp/work")) + + def test_build_process_output_details_keeps_partial_timeout_logs(self): + module = load_validator_module() + + details = module.build_process_output_details( + stdout="line one\nline two\n", + stderr=b"warning\n", + ) + + self.assertEqual(details, {"stdout": "line one\nline two", "stderr": "warning"}) + class MetadataValidationTests(unittest.TestCase): def test_reports_missing_required_metadata_fields(self): From 4ade5f65531b3157177a4490a86dae38e816e354 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=82=B9=E6=B0=B8=E8=B5=AB?= <1259085392@qq.com> Date: Fri, 24 Apr 2026 12:05:16 +0900 Subject: [PATCH 04/15] handle invalid plugin metadata gracefully --- scripts/validate_plugins/run.py | 28 ++++++++++++++++++++++++++-- tests/test_validate_plugins.py | 18 ++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/scripts/validate_plugins/run.py b/scripts/validate_plugins/run.py index ebf8ee23..762f238d 100644 --- a/scripts/validate_plugins/run.py +++ b/scripts/validate_plugins/run.py @@ -24,6 +24,11 @@ REQUIRED_METADATA_FIELDS = ("name", "desc", "version", "author") DEFAULT_CLONE_TIMEOUT = 120 +CONFLICT_MARKERS = ("<<<<<<<", "=======", ">>>>>>>") + + +class MetadataLoadError(ValueError): + pass def build_result( @@ -104,8 +109,17 @@ def _parse_simple_yaml(path: Path) -> dict: def load_metadata(path: Path) -> dict: + text = path.read_text(encoding="utf-8") + if any(marker in text for marker in CONFLICT_MARKERS): + raise MetadataLoadError( + "could not find expected ':' (merge conflict markers found in metadata.yaml)" + ) + if yaml is not None: - loaded = yaml.safe_load(path.read_text(encoding="utf-8")) + try: + loaded = yaml.safe_load(text) + except yaml.YAMLError as exc: + raise MetadataLoadError(str(exc)) from exc if isinstance(loaded, dict): return loaded return {} @@ -121,7 +135,16 @@ def precheck_plugin_directory(plugin_dir: Path) -> dict: "message": "missing metadata.yaml", } - metadata = load_metadata(metadata_path) + try: + metadata = load_metadata(metadata_path) + except MetadataLoadError as exc: + return { + "ok": False, + "stage": "metadata", + "message": "invalid metadata.yaml", + "details": str(exc), + } + missing = [ field for field in REQUIRED_METADATA_FIELDS @@ -367,6 +390,7 @@ def validate_plugin( ok=False, stage=precheck["stage"], message=precheck["message"], + details=precheck.get("details"), ) plugin_dir_name = precheck["plugin_dir_name"] diff --git a/tests/test_validate_plugins.py b/tests/test_validate_plugins.py index 18ab69da..8316c373 100644 --- a/tests/test_validate_plugins.py +++ b/tests/test_validate_plugins.py @@ -215,6 +215,24 @@ def test_reports_missing_required_metadata_fields(self): self.assertIn("desc", result["message"]) self.assertIn("version", result["message"]) + def test_reports_invalid_metadata_yaml_without_raising(self): + module = load_validator_module() + + with tempfile.TemporaryDirectory() as tmp_dir: + plugin_dir = Path(tmp_dir) + (plugin_dir / "metadata.yaml").write_text( + "name: demo_plugin\n<<<<<<< HEAD\ndesc: broken\n=======\ndesc: fixed\n>>>>>>> branch\n", + encoding="utf-8", + ) + (plugin_dir / "main.py").write_text("print('hello')\n", encoding="utf-8") + + result = module.precheck_plugin_directory(plugin_dir) + + self.assertFalse(result["ok"]) + self.assertEqual(result["stage"], "metadata") + self.assertIn("invalid metadata.yaml", result["message"]) + self.assertIn("could not find expected ':'", result["details"]) + class WorkerCommandTests(unittest.TestCase): def test_build_worker_command_contains_required_arguments(self): From 0fd923e99c044a019ab1e4e845b9b835c561910e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=82=B9=E6=B0=B8=E8=B5=AB?= <1259085392@qq.com> Date: Fri, 24 Apr 2026 12:13:03 +0900 Subject: [PATCH 05/15] improve plugin validation diagnostics --- .github/workflows/validate-plugin-smoke.yml | 9 +- scripts/validate_plugins/run.py | 83 ++++++++--- tests/test_validate_plugins.py | 151 ++++++++++++++++++++ 3 files changed, 223 insertions(+), 20 deletions(-) diff --git a/.github/workflows/validate-plugin-smoke.yml b/.github/workflows/validate-plugin-smoke.yml index bb9d1470..e131b92d 100644 --- a/.github/workflows/validate-plugin-smoke.yml +++ b/.github/workflows/validate-plugin-smoke.yml @@ -46,6 +46,7 @@ jobs: import json import os import subprocess + import sys from pathlib import Path base_ref = os.environ["GITHUB_BASE_REF"] @@ -70,9 +71,11 @@ jobs: if not isinstance(head, dict): raise ValueError("plugins.json must contain a JSON object") except (json.JSONDecodeError, ValueError) as exc: - changed = [] - should_validate = False - validation_note = f"Skipping smoke validation because plugins.json is invalid on the PR head: {exc}" + print( + f"plugins.json is invalid on the PR head; smoke validation cannot continue: {exc}", + file=sys.stderr, + ) + raise SystemExit(1) else: changed = [ name diff --git a/scripts/validate_plugins/run.py b/scripts/validate_plugins/run.py index 762f238d..06b36feb 100644 --- a/scripts/validate_plugins/run.py +++ b/scripts/validate_plugins/run.py @@ -98,13 +98,27 @@ def select_plugins( def _parse_simple_yaml(path: Path) -> dict: + def parse_value(raw_value: str) -> str: + value = raw_value.strip() + if not value: + return "" + + if value[0] in {'"', "'"}: + quote = value[0] + end_index = value.rfind(quote) + if end_index > 0: + return value[1:end_index] + + value = re.split(r"\s+#", value, maxsplit=1)[0].rstrip() + return value.strip("\"'") + result = {} for raw_line in path.read_text(encoding="utf-8").splitlines(): line = raw_line.strip() if not line or line.startswith("#") or ":" not in line: continue key, value = line.split(":", 1) - result[key.strip()] = value.strip().strip("\"'") + result[key.strip()] = parse_value(value) return result @@ -219,11 +233,17 @@ def load_plugins_index(path: Path) -> dict[str, dict]: data = json.loads(path.read_text(encoding="utf-8")) if not isinstance(data, dict): raise ValueError("plugins.json must contain a JSON object") - result = {} + for key, value in data.items(): - if isinstance(key, str) and isinstance(value, dict): - result[key] = value - return result + if not isinstance(key, str): + raise ValueError( + f"plugins.json keys must be strings, got {type(key).__name__!r}: {key!r}" + ) + if not isinstance(value, dict): + raise ValueError( + f"plugins.json values must be objects/dicts; key {key!r} has {type(value).__name__!r}" + ) + return data def combine_requested_names( @@ -431,6 +451,39 @@ def validate_plugin( ) +def validate_selected_plugins( + *, + selected: list[tuple[str, dict]], + astrbot_path: Path, + script_path: Path, + work_dir: Path, + clone_timeout: int, + load_timeout: int, +) -> list[dict]: + results = [] + total = len(selected) + + for index, (plugin, plugin_data) in enumerate(selected, start=1): + print(f"[{index}/{total}] Validating {plugin}", flush=True) + result = validate_plugin( + plugin=plugin, + plugin_data=plugin_data, + astrbot_path=astrbot_path, + script_path=script_path, + work_dir=work_dir, + clone_timeout=clone_timeout, + load_timeout=load_timeout, + ) + results.append(result) + + status = "PASS" if result.get("ok") else "FAIL" + stage = result.get("stage", "unknown") + message = result.get("message", "") + print(f"[{index}/{total}] {status} {plugin} [{stage}] {message}", flush=True) + + return results + + class NullStub: def __getattr__(self, name: str) -> "NullStub": del name @@ -646,18 +699,14 @@ def main() -> int: work_dir.mkdir(parents=True, exist_ok=True) try: - results = [ - validate_plugin( - plugin=plugin, - plugin_data=plugin_data, - astrbot_path=Path(args.astrbot_path).resolve(), - script_path=Path(__file__).resolve(), - work_dir=work_dir, - clone_timeout=args.clone_timeout, - load_timeout=args.load_timeout, - ) - for plugin, plugin_data in selected - ] + results = validate_selected_plugins( + selected=selected, + astrbot_path=Path(args.astrbot_path).resolve(), + script_path=Path(__file__).resolve(), + work_dir=work_dir, + clone_timeout=args.clone_timeout, + load_timeout=args.load_timeout, + ) finally: if temp_dir is not None: temp_dir.cleanup() diff --git a/tests/test_validate_plugins.py b/tests/test_validate_plugins.py index 8316c373..c8590cc7 100644 --- a/tests/test_validate_plugins.py +++ b/tests/test_validate_plugins.py @@ -1,9 +1,12 @@ import importlib.util +import json +import os import subprocess import sys import tempfile import unittest from pathlib import Path +from unittest import mock ROOT = Path(__file__).resolve().parents[1] @@ -195,6 +198,154 @@ def test_build_process_output_details_keeps_partial_timeout_logs(self): self.assertEqual(details, {"stdout": "line one\nline two", "stderr": "warning"}) + def test_parse_simple_yaml_handles_comments_quotes_and_whitespace(self): + module = load_validator_module() + + with tempfile.NamedTemporaryFile("w", suffix=".yml", delete=False) as handle: + handle.write( + """ + # leading comment + + key1: value1 # trailing comment + key2: \" spaced value \" + key3: 'another value' + key4: value-with-#-hash + """ + ) + metadata_path = Path(handle.name) + + try: + parsed = module._parse_simple_yaml(metadata_path) + finally: + os.remove(metadata_path) + + self.assertEqual(parsed["key1"], "value1") + self.assertEqual(parsed["key2"], " spaced value ") + self.assertEqual(parsed["key3"], "another value") + self.assertEqual(parsed["key4"], "value-with-#-hash") + + def test_load_metadata_uses_yaml_safe_load_when_available(self): + module = load_validator_module() + + with tempfile.NamedTemporaryFile("w", suffix=".yml", delete=False) as handle: + handle.write("name: should-be-overridden\n") + metadata_path = Path(handle.name) + + fake_yaml = mock.Mock() + fake_yaml.safe_load.return_value = {"name": "from-yaml", "version": "1.0.0"} + + try: + with mock.patch.object(module, "yaml", fake_yaml): + metadata = module.load_metadata(metadata_path) + finally: + os.remove(metadata_path) + + self.assertEqual(metadata, {"name": "from-yaml", "version": "1.0.0"}) + fake_yaml.safe_load.assert_called_once() + + def test_load_metadata_uses_simple_parser_when_yaml_unavailable(self): + module = load_validator_module() + + with tempfile.NamedTemporaryFile("w", suffix=".yml", delete=False) as handle: + handle.write( + """ + name: demo-plugin + version: \"0.2.3\" + """ + ) + metadata_path = Path(handle.name) + + yaml_backup = getattr(module, "yaml", None) + try: + module.yaml = None + metadata = module.load_metadata(metadata_path) + finally: + module.yaml = yaml_backup + os.remove(metadata_path) + + self.assertEqual(metadata.get("name"), "demo-plugin") + self.assertEqual(metadata.get("version"), "0.2.3") + + def test_load_plugins_index_accepts_valid_object(self): + module = load_validator_module() + + index_obj = { + "good-plugin": {"name": "Good Plugin", "repo": "https://github.com/example/good"}, + "another-plugin": {"name": "Another Plugin"}, + } + + with tempfile.NamedTemporaryFile("w", suffix=".json", delete=False) as handle: + json.dump(index_obj, handle) + index_path = Path(handle.name) + + try: + plugins = module.load_plugins_index(index_path) + finally: + os.remove(index_path) + + self.assertEqual(plugins, index_obj) + + def test_load_plugins_index_rejects_json_array(self): + module = load_validator_module() + + with tempfile.NamedTemporaryFile("w", suffix=".json", delete=False) as handle: + json.dump([{"name": "array-entry"}], handle) + index_path = Path(handle.name) + + try: + with self.assertRaisesRegex(ValueError, "plugins.json must contain a JSON object"): + module.load_plugins_index(index_path) + finally: + os.remove(index_path) + + def test_load_plugins_index_rejects_non_dict_values(self): + module = load_validator_module() + + index_obj = { + "valid-plugin": {"name": "Valid Plugin", "repo": "https://github.com/example/valid"}, + "not-a-dict": "just-a-string", + } + + with tempfile.NamedTemporaryFile("w", suffix=".json", delete=False) as handle: + json.dump(index_obj, handle) + index_path = Path(handle.name) + + try: + with self.assertRaisesRegex(ValueError, "plugins.json values must be objects/dicts"): + module.load_plugins_index(index_path) + finally: + os.remove(index_path) + + +class ValidationProgressTests(unittest.TestCase): + def test_validate_selected_plugins_emits_progress_and_result_lines(self): + module = load_validator_module() + selected = [ + ("plugin-a", {"repo": "https://github.com/example/plugin-a"}), + ("plugin-b", {"repo": "https://github.com/example/plugin-b"}), + ] + fake_results = [ + {"plugin": "plugin-a", "ok": True, "stage": "load", "message": "ok"}, + {"plugin": "plugin-b", "ok": False, "stage": "metadata", "message": "invalid metadata.yaml"}, + ] + + with mock.patch.object(module, "validate_plugin", side_effect=fake_results) as validate_mock: + with mock.patch("builtins.print") as print_mock: + results = module.validate_selected_plugins( + selected=selected, + astrbot_path=Path("/tmp/AstrBot"), + script_path=Path("/tmp/run.py"), + work_dir=Path("/tmp/work"), + clone_timeout=60, + load_timeout=300, + ) + + self.assertEqual(results, fake_results) + self.assertEqual(validate_mock.call_count, 2) + print_mock.assert_any_call("[1/2] Validating plugin-a", flush=True) + print_mock.assert_any_call("[1/2] PASS plugin-a [load] ok", flush=True) + print_mock.assert_any_call("[2/2] FAIL plugin-b [metadata] invalid metadata.yaml", flush=True) + class MetadataValidationTests(unittest.TestCase): def test_reports_missing_required_metadata_fields(self): From e2d69fdaea530cc016864610f6dcd1f4362e887b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=82=B9=E6=B0=B8=E8=B5=AB?= <1259085392@qq.com> Date: Fri, 24 Apr 2026 12:43:49 +0900 Subject: [PATCH 06/15] tighten plugin validation error paths --- .github/workflows/validate-plugin-smoke.yml | 16 ++- scripts/validate_plugins/run.py | 8 +- tests/test_validate_plugins.py | 132 ++++++++++++++++++++ 3 files changed, 152 insertions(+), 4 deletions(-) diff --git a/.github/workflows/validate-plugin-smoke.yml b/.github/workflows/validate-plugin-smoke.yml index e131b92d..383bdaf8 100644 --- a/.github/workflows/validate-plugin-smoke.yml +++ b/.github/workflows/validate-plugin-smoke.yml @@ -86,8 +86,22 @@ jobs: if not should_validate: validation_note = "No plugin entries changed in plugins.json; skipping smoke validation." + astrbot_ref = "master" + try: + default_head = subprocess.check_output( + ["git", "ls-remote", "--symref", "https://github.com/AstrBotDevs/AstrBot", "HEAD"], + text=True, + stderr=subprocess.DEVNULL, + ) + for line in default_head.splitlines(): + if line.startswith("ref: refs/heads/") and line.endswith("\tHEAD"): + astrbot_ref = line.split("refs/heads/", 1)[1].split("\t", 1)[0] + break + except subprocess.CalledProcessError: + pass + with open(os.environ["GITHUB_ENV"], "a", encoding="utf-8") as handle: - handle.write("ASTRBOT_REF=master\n") + handle.write(f"ASTRBOT_REF={astrbot_ref}\n") handle.write(f"PLUGIN_NAME_LIST={','.join(changed)}\n") handle.write("PLUGIN_LIMIT=\n") handle.write(f"SHOULD_VALIDATE={'true' if should_validate else 'false'}\n") diff --git a/scripts/validate_plugins/run.py b/scripts/validate_plugins/run.py index 06b36feb..b7773ac2 100644 --- a/scripts/validate_plugins/run.py +++ b/scripts/validate_plugins/run.py @@ -134,9 +134,11 @@ def load_metadata(path: Path) -> dict: loaded = yaml.safe_load(text) except yaml.YAMLError as exc: raise MetadataLoadError(str(exc)) from exc - if isinstance(loaded, dict): - return loaded - return {} + if loaded is None: + return {} + if not isinstance(loaded, dict): + raise MetadataLoadError("metadata.yaml must contain a mapping at the top level") + return loaded return _parse_simple_yaml(path) diff --git a/tests/test_validate_plugins.py b/tests/test_validate_plugins.py index c8590cc7..b437979f 100644 --- a/tests/test_validate_plugins.py +++ b/tests/test_validate_plugins.py @@ -243,6 +243,27 @@ def test_load_metadata_uses_yaml_safe_load_when_available(self): self.assertEqual(metadata, {"name": "from-yaml", "version": "1.0.0"}) fake_yaml.safe_load.assert_called_once() + def test_load_metadata_rejects_non_mapping_yaml_root(self): + module = load_validator_module() + + with tempfile.NamedTemporaryFile("w", suffix=".yml", delete=False) as handle: + handle.write("- item\n") + metadata_path = Path(handle.name) + + fake_yaml = mock.Mock() + fake_yaml.safe_load.return_value = ["item"] + fake_yaml.YAMLError = ValueError + + try: + with mock.patch.object(module, "yaml", fake_yaml): + with self.assertRaisesRegex( + module.MetadataLoadError, + "metadata.yaml must contain a mapping at the top level", + ): + module.load_metadata(metadata_path) + finally: + os.remove(metadata_path) + def test_load_metadata_uses_simple_parser_when_yaml_unavailable(self): module = load_validator_module() @@ -347,6 +368,117 @@ def test_validate_selected_plugins_emits_progress_and_result_lines(self): print_mock.assert_any_call("[2/2] FAIL plugin-b [metadata] invalid metadata.yaml", flush=True) +class ValidatePluginTests(unittest.TestCase): + def setUp(self): + self.module = load_validator_module() + self.plugin_key = "demo-plugin" + self.plugin_data = {"repo": "https://github.com/example/demo-plugin"} + self.astrbot_path = Path("/tmp/AstrBot") + self.script_path = Path("/tmp/run.py") + self.work_dir = Path("/tmp/work") + + def call_validate_plugin(self, plugin_data=None): + return self.module.validate_plugin( + plugin=self.plugin_key, + plugin_data=self.plugin_data if plugin_data is None else plugin_data, + astrbot_path=self.astrbot_path, + script_path=self.script_path, + work_dir=self.work_dir, + clone_timeout=30, + load_timeout=60, + ) + + def test_missing_repo_field_sets_repo_url_stage(self): + result = self.call_validate_plugin(plugin_data={}) + + self.assertFalse(result["ok"]) + self.assertEqual(result["stage"], "repo_url") + self.assertEqual(result["message"], "missing repo field") + + def test_invalid_repo_url_sets_repo_url_stage(self): + with mock.patch.object(self.module, "normalize_repo_url", side_effect=ValueError("invalid repo URL")): + result = self.call_validate_plugin() + + self.assertFalse(result["ok"]) + self.assertEqual(result["stage"], "repo_url") + self.assertEqual(result["message"], "invalid repo URL") + + def test_clone_called_process_error_uses_stderr_or_stdout(self): + error = subprocess.CalledProcessError( + returncode=1, + cmd=["git", "clone"], + output="clone stdout", + stderr="clone stderr", + ) + + with mock.patch.object(self.module, "clone_plugin_repo", side_effect=error): + result = self.call_validate_plugin() + + self.assertFalse(result["ok"]) + self.assertEqual(result["stage"], "clone") + self.assertIn("clone stderr", result["message"]) + + def test_clone_timeout_uses_process_output_details(self): + timeout = subprocess.TimeoutExpired(cmd=["git", "clone"], timeout=30, output="slow", stderr="warning") + + with mock.patch.object(self.module, "clone_plugin_repo", side_effect=timeout): + with mock.patch.object( + self.module, + "build_process_output_details", + return_value={"stdout": "slow", "stderr": "warning"}, + ) as details_mock: + result = self.call_validate_plugin() + + self.assertFalse(result["ok"]) + self.assertEqual(result["stage"], "clone_timeout") + self.assertEqual(result["details"], {"stdout": "slow", "stderr": "warning"}) + details_mock.assert_called_once_with(stdout="slow", stderr="warning") + + def test_precheck_failure_is_mapped_into_result(self): + with mock.patch.object(self.module, "clone_plugin_repo"): + with mock.patch.object( + self.module, + "precheck_plugin_directory", + return_value={"ok": False, "stage": "metadata", "message": "invalid metadata", "details": "line 3"}, + ): + result = self.call_validate_plugin() + + self.assertFalse(result["ok"]) + self.assertEqual(result["stage"], "metadata") + self.assertEqual(result["message"], "invalid metadata") + self.assertEqual(result["details"], "line 3") + + def test_successful_clone_and_precheck_invokes_worker_and_parses_output(self): + completed = subprocess.CompletedProcess( + args=["python3", "run.py"], + returncode=0, + stdout='{"ok": true}', + stderr="", + ) + parsed_output = {"ok": True, "stage": "load", "message": "plugin loaded successfully"} + + with mock.patch.object( + self.module, + "precheck_plugin_directory", + return_value={"ok": True, "plugin_dir_name": "demo_plugin", "message": "ok", "stage": "precheck"}, + ) as precheck_mock: + with mock.patch.object(self.module, "clone_plugin_repo"): + with mock.patch.object(subprocess, "run", return_value=completed) as run_mock: + with mock.patch.object(self.module, "parse_worker_output", return_value=parsed_output) as parse_mock: + result = self.call_validate_plugin() + + self.assertEqual(result, parsed_output) + precheck_mock.assert_called_once() + run_mock.assert_called_once() + parse_mock.assert_called_once_with( + plugin=self.plugin_key, + repo=self.plugin_data["repo"], + normalized_repo_url=self.plugin_data["repo"], + completed=completed, + plugin_dir_name="demo_plugin", + ) + + class MetadataValidationTests(unittest.TestCase): def test_reports_missing_required_metadata_fields(self): module = load_validator_module() From f0a0a55923cbbba50c03a4839b7cb23c601e432d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=82=B9=E6=B0=B8=E8=B5=AB?= <1259085392@qq.com> Date: Fri, 24 Apr 2026 12:57:47 +0900 Subject: [PATCH 07/15] extract pull request detection helper --- .github/workflows/validate-plugin-smoke.yml | 67 +--------- .../detect_changed_plugins.py | 123 ++++++++++++++++++ tests/test_detect_changed_plugins.py | 123 ++++++++++++++++++ 3 files changed, 247 insertions(+), 66 deletions(-) create mode 100644 scripts/validate_plugins/detect_changed_plugins.py create mode 100644 tests/test_detect_changed_plugins.py diff --git a/.github/workflows/validate-plugin-smoke.yml b/.github/workflows/validate-plugin-smoke.yml index 383bdaf8..6884b589 100644 --- a/.github/workflows/validate-plugin-smoke.yml +++ b/.github/workflows/validate-plugin-smoke.yml @@ -41,72 +41,7 @@ jobs: - name: Detect changed plugins from pull request if: github.event_name == 'pull_request' - run: | - python - <<'PY' - import json - import os - import subprocess - import sys - from pathlib import Path - - base_ref = os.environ["GITHUB_BASE_REF"] - subprocess.run(["git", "fetch", "origin", base_ref, "--depth", "1"], check=True) - - validation_note = "" - - try: - base_raw = subprocess.check_output( - ["git", "show", f"origin/{base_ref}:plugins.json"], - text=True, - stderr=subprocess.DEVNULL, - ) - base = json.loads(base_raw) - if not isinstance(base, dict): - base = {} - except (subprocess.CalledProcessError, json.JSONDecodeError): - base = {} - - try: - head = json.loads(Path("plugins.json").read_text(encoding="utf-8")) - if not isinstance(head, dict): - raise ValueError("plugins.json must contain a JSON object") - except (json.JSONDecodeError, ValueError) as exc: - print( - f"plugins.json is invalid on the PR head; smoke validation cannot continue: {exc}", - file=sys.stderr, - ) - raise SystemExit(1) - else: - changed = [ - name - for name, payload in head.items() - if base.get(name) != payload - ] - should_validate = bool(changed) - if not should_validate: - validation_note = "No plugin entries changed in plugins.json; skipping smoke validation." - - astrbot_ref = "master" - try: - default_head = subprocess.check_output( - ["git", "ls-remote", "--symref", "https://github.com/AstrBotDevs/AstrBot", "HEAD"], - text=True, - stderr=subprocess.DEVNULL, - ) - for line in default_head.splitlines(): - if line.startswith("ref: refs/heads/") and line.endswith("\tHEAD"): - astrbot_ref = line.split("refs/heads/", 1)[1].split("\t", 1)[0] - break - except subprocess.CalledProcessError: - pass - - with open(os.environ["GITHUB_ENV"], "a", encoding="utf-8") as handle: - handle.write(f"ASTRBOT_REF={astrbot_ref}\n") - handle.write(f"PLUGIN_NAME_LIST={','.join(changed)}\n") - handle.write("PLUGIN_LIMIT=\n") - handle.write(f"SHOULD_VALIDATE={'true' if should_validate else 'false'}\n") - handle.write(f"VALIDATION_NOTE={validation_note}\n") - PY + run: python scripts/validate_plugins/detect_changed_plugins.py - name: Show PR diff selection if: github.event_name == 'pull_request' diff --git a/scripts/validate_plugins/detect_changed_plugins.py b/scripts/validate_plugins/detect_changed_plugins.py new file mode 100644 index 00000000..8187020d --- /dev/null +++ b/scripts/validate_plugins/detect_changed_plugins.py @@ -0,0 +1,123 @@ +#!/usr/bin/env python3 + +from __future__ import annotations + +import json +import os +import subprocess +import sys +from pathlib import Path + + +DEFAULT_ASTRBOT_REF = "master" +ASTRBOT_REMOTE_URL = "https://github.com/AstrBotDevs/AstrBot" + + +def load_plugins_map(text: str, *, source_name: str) -> dict[str, dict]: + try: + data = json.loads(text) + except json.JSONDecodeError as exc: + raise ValueError(f"plugins.json is invalid on the {source_name}: {exc}") from exc + + if not isinstance(data, dict): + raise ValueError("plugins.json must contain a JSON object") + return data + + +def detect_changed_plugin_names(*, base: dict[str, dict], head: dict[str, dict]) -> list[str]: + return [name for name, payload in head.items() if base.get(name) != payload] + + +def fetch_base_ref(base_ref: str) -> None: + subprocess.run(["git", "fetch", "origin", base_ref, "--depth", "1"], check=True) + + +def read_base_plugins_json(base_ref: str) -> str: + return subprocess.check_output( + ["git", "show", f"origin/{base_ref}:plugins.json"], + text=True, + stderr=subprocess.DEVNULL, + ) + + +def resolve_astrbot_ref() -> str: + try: + default_head = subprocess.check_output( + ["git", "ls-remote", "--symref", ASTRBOT_REMOTE_URL, "HEAD"], + text=True, + stderr=subprocess.DEVNULL, + ) + except subprocess.CalledProcessError: + return DEFAULT_ASTRBOT_REF + + for line in default_head.splitlines(): + if line.startswith("ref: refs/heads/") and line.endswith("\tHEAD"): + return line.split("refs/heads/", 1)[1].split("\t", 1)[0] + return DEFAULT_ASTRBOT_REF + + +def detect_pull_request_selection(*, repo_root: Path, base_ref: str) -> dict[str, object]: + fetch_base_ref(base_ref) + + try: + base = load_plugins_map(read_base_plugins_json(base_ref), source_name=f"base ref {base_ref}") + except (subprocess.CalledProcessError, ValueError): + base = {} + + head_text = (repo_root / "plugins.json").read_text(encoding="utf-8") + try: + head = load_plugins_map(head_text, source_name="PR head") + except ValueError as exc: + raise ValueError(f"plugins.json is invalid on the PR head: {exc}") from exc + + changed = detect_changed_plugin_names(base=base, head=head) + validation_note = "" + if not changed: + validation_note = "No plugin entries changed in plugins.json; skipping smoke validation." + + return { + "changed": changed, + "should_validate": bool(changed), + "validation_note": validation_note, + } + + +def write_github_env( + *, + env_path: Path, + astrbot_ref: str, + changed: list[str], + should_validate: bool, + validation_note: str, +) -> None: + with env_path.open("a", encoding="utf-8") as handle: + handle.write(f"ASTRBOT_REF={astrbot_ref}\n") + handle.write(f"PLUGIN_NAME_LIST={','.join(changed)}\n") + handle.write("PLUGIN_LIMIT=\n") + handle.write(f"SHOULD_VALIDATE={'true' if should_validate else 'false'}\n") + handle.write(f"VALIDATION_NOTE={validation_note}\n") + + +def main() -> int: + base_ref = os.environ["GITHUB_BASE_REF"] + github_env = Path(os.environ["GITHUB_ENV"]) + repo_root = Path.cwd() + + try: + result = detect_pull_request_selection(repo_root=repo_root, base_ref=base_ref) + except ValueError as exc: + print(str(exc), file=sys.stderr) + return 1 + + write_github_env( + env_path=github_env, + astrbot_ref=resolve_astrbot_ref(), + changed=result["changed"], + should_validate=result["should_validate"], + validation_note=result["validation_note"], + ) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/test_detect_changed_plugins.py b/tests/test_detect_changed_plugins.py new file mode 100644 index 00000000..cb807b95 --- /dev/null +++ b/tests/test_detect_changed_plugins.py @@ -0,0 +1,123 @@ +import importlib.util +import tempfile +import unittest +from pathlib import Path +from unittest import mock + + +ROOT = Path(__file__).resolve().parents[1] +MODULE_PATH = ROOT / "scripts" / "validate_plugins" / "detect_changed_plugins.py" + + +def load_detection_module(): + if not MODULE_PATH.exists(): + raise AssertionError(f"detection script missing: {MODULE_PATH}") + + spec = importlib.util.spec_from_file_location("detect_changed_plugins", MODULE_PATH) + if spec is None or spec.loader is None: + raise AssertionError("unable to load detection module spec") + + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +class LoadPluginsMapTests(unittest.TestCase): + def test_load_plugins_map_requires_json_object(self): + module = load_detection_module() + + with self.assertRaisesRegex(ValueError, "plugins.json must contain a JSON object"): + module.load_plugins_map('[{"name": "bad"}]', source_name="head") + + def test_load_plugins_map_returns_dict_for_valid_json(self): + module = load_detection_module() + + plugins = module.load_plugins_map('{"plugin-a": {"repo": "https://github.com/example/a"}}', source_name="head") + + self.assertEqual(plugins, {"plugin-a": {"repo": "https://github.com/example/a"}}) + + +class ChangedPluginDetectionTests(unittest.TestCase): + def test_detect_changed_plugin_names_returns_only_modified_entries(self): + module = load_detection_module() + + changed = module.detect_changed_plugin_names( + base={"plugin-a": {"repo": "a"}, "plugin-b": {"repo": "b"}}, + head={"plugin-a": {"repo": "a"}, "plugin-b": {"repo": "changed"}, "plugin-c": {"repo": "c"}}, + ) + + self.assertEqual(changed, ["plugin-b", "plugin-c"]) + + +class AstrbotRefTests(unittest.TestCase): + def test_resolve_astrbot_ref_uses_remote_default_branch(self): + module = load_detection_module() + + with mock.patch.object(module.subprocess, "check_output", return_value="ref: refs/heads/main\tHEAD\nabc\tHEAD\n"): + ref = module.resolve_astrbot_ref() + + self.assertEqual(ref, "main") + + def test_resolve_astrbot_ref_falls_back_to_master(self): + module = load_detection_module() + + with mock.patch.object(module.subprocess, "check_output", side_effect=module.subprocess.CalledProcessError(1, ["git"])): + ref = module.resolve_astrbot_ref() + + self.assertEqual(ref, "master") + + +class PullRequestDetectionTests(unittest.TestCase): + def test_detect_pull_request_selection_handles_missing_base_file(self): + module = load_detection_module() + + with tempfile.TemporaryDirectory() as tmp_dir: + repo_root = Path(tmp_dir) + plugins_json = repo_root / "plugins.json" + plugins_json.write_text('{"plugin-a": {"repo": "https://github.com/example/a"}}', encoding="utf-8") + + with mock.patch.object(module, "fetch_base_ref") as fetch_mock: + with mock.patch.object(module, "read_base_plugins_json", side_effect=module.subprocess.CalledProcessError(1, ["git"])): + result = module.detect_pull_request_selection(repo_root=repo_root, base_ref="main") + + fetch_mock.assert_called_once_with("main") + self.assertEqual(result["changed"], ["plugin-a"]) + self.assertEqual(result["validation_note"], "") + + def test_detect_pull_request_selection_raises_on_invalid_head_json(self): + module = load_detection_module() + + with tempfile.TemporaryDirectory() as tmp_dir: + repo_root = Path(tmp_dir) + (repo_root / "plugins.json").write_text('{bad json', encoding="utf-8") + + with mock.patch.object(module, "fetch_base_ref"): + with mock.patch.object(module, "read_base_plugins_json", return_value='{}'): + with self.assertRaisesRegex(ValueError, "plugins.json is invalid on the PR head"): + module.detect_pull_request_selection(repo_root=repo_root, base_ref="main") + + def test_write_github_env_outputs_expected_values(self): + module = load_detection_module() + + with tempfile.NamedTemporaryFile("w+", delete=False) as handle: + env_path = Path(handle.name) + + try: + module.write_github_env( + env_path=env_path, + astrbot_ref="master", + changed=["plugin-a", "plugin-b"], + should_validate=True, + validation_note="", + ) + content = env_path.read_text(encoding="utf-8") + finally: + env_path.unlink(missing_ok=True) + + self.assertIn("ASTRBOT_REF=master\n", content) + self.assertIn("PLUGIN_NAME_LIST=plugin-a,plugin-b\n", content) + self.assertIn("SHOULD_VALIDATE=true\n", content) + + +if __name__ == "__main__": + unittest.main() From 096813c2da03f50cdfbad684ba2330a55e3fb35d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=82=B9=E6=B0=B8=E8=B5=AB?= <1259085392@qq.com> Date: Fri, 24 Apr 2026 13:13:36 +0900 Subject: [PATCH 08/15] tighten metadata and plugin map parsing --- .../detect_changed_plugins.py | 14 ++-- scripts/validate_plugins/plugins_map.py | 34 ++++++++ scripts/validate_plugins/run.py | 68 +++++++++++----- tests/test_detect_changed_plugins.py | 6 ++ tests/test_validate_plugins.py | 77 +++++++++++++++---- 5 files changed, 156 insertions(+), 43 deletions(-) create mode 100644 scripts/validate_plugins/plugins_map.py diff --git a/scripts/validate_plugins/detect_changed_plugins.py b/scripts/validate_plugins/detect_changed_plugins.py index 8187020d..3e8584b3 100644 --- a/scripts/validate_plugins/detect_changed_plugins.py +++ b/scripts/validate_plugins/detect_changed_plugins.py @@ -8,20 +8,18 @@ import sys from pathlib import Path +if __package__ in {None, ""}: + sys.path.insert(0, str(Path(__file__).resolve().parents[2])) + +from scripts.validate_plugins.plugins_map import load_plugins_map_text + DEFAULT_ASTRBOT_REF = "master" ASTRBOT_REMOTE_URL = "https://github.com/AstrBotDevs/AstrBot" def load_plugins_map(text: str, *, source_name: str) -> dict[str, dict]: - try: - data = json.loads(text) - except json.JSONDecodeError as exc: - raise ValueError(f"plugins.json is invalid on the {source_name}: {exc}") from exc - - if not isinstance(data, dict): - raise ValueError("plugins.json must contain a JSON object") - return data + return load_plugins_map_text(text, source_name=source_name) def detect_changed_plugin_names(*, base: dict[str, dict], head: dict[str, dict]) -> list[str]: diff --git a/scripts/validate_plugins/plugins_map.py b/scripts/validate_plugins/plugins_map.py new file mode 100644 index 00000000..07b689cd --- /dev/null +++ b/scripts/validate_plugins/plugins_map.py @@ -0,0 +1,34 @@ +from __future__ import annotations + +import json +from pathlib import Path + + +def validate_plugins_map(data: object, *, source_name: str) -> dict[str, dict]: + if not isinstance(data, dict): + raise ValueError("plugins.json must contain a JSON object") + + for name, payload in data.items(): + if not isinstance(name, str): + raise ValueError( + f"plugins.json on the {source_name} has a non-string key: {name!r}" + ) + if not isinstance(payload, dict): + raise ValueError( + f"plugins.json entry {name!r} on the {source_name} must be a JSON object" + ) + + return data + + +def load_plugins_map_text(text: str, *, source_name: str) -> dict[str, dict]: + try: + data = json.loads(text) + except json.JSONDecodeError as exc: + raise ValueError(f"plugins.json is invalid on the {source_name}: {exc}") from exc + + return validate_plugins_map(data, source_name=source_name) + + +def load_plugins_map_file(path: Path, *, source_name: str) -> dict[str, dict]: + return load_plugins_map_text(path.read_text(encoding="utf-8"), source_name=source_name) diff --git a/scripts/validate_plugins/run.py b/scripts/validate_plugins/run.py index b7773ac2..768bdd96 100644 --- a/scripts/validate_plugins/run.py +++ b/scripts/validate_plugins/run.py @@ -16,6 +16,11 @@ from pathlib import Path from urllib.parse import urlparse +if __package__ in {None, ""}: + sys.path.insert(0, str(Path(__file__).resolve().parents[2])) + +from scripts.validate_plugins.plugins_map import load_plugins_map_file + try: import yaml except ImportError: # pragma: no cover - optional in local unit tests @@ -98,6 +103,15 @@ def select_plugins( def _parse_simple_yaml(path: Path) -> dict: + """Very small YAML subset parser used as a fallback when PyYAML is unavailable. + + Supported format: + - Flat mapping of `key: value` pairs + - No indentation (no nested objects or multiline continuations) + - No lists (`- item` syntax) + - `#` starts a comment when preceded by whitespace (or at line start) + """ + def parse_value(raw_value: str) -> str: value = raw_value.strip() if not value: @@ -112,13 +126,36 @@ def parse_value(raw_value: str) -> str: value = re.split(r"\s+#", value, maxsplit=1)[0].rstrip() return value.strip("\"'") - result = {} - for raw_line in path.read_text(encoding="utf-8").splitlines(): - line = raw_line.strip() - if not line or line.startswith("#") or ":" not in line: + result: dict[str, str] = {} + for lineno, raw_line in enumerate(path.read_text(encoding="utf-8").splitlines(), start=1): + stripped = raw_line.strip() + if not stripped: + continue + if stripped.startswith("#"): continue + if raw_line[0].isspace(): + raise ValueError( + f"Unsupported YAML indentation in {path} at line {lineno}: {raw_line!r}" + ) + + line = stripped + if line.startswith("-"): + raise ValueError( + f"Unsupported YAML list syntax in {path} at line {lineno}: {raw_line!r}" + ) + if ":" not in line: + raise ValueError( + f"Unsupported YAML content (expected 'key: value') in {path} at line {lineno}: {raw_line!r}" + ) + key, value = line.split(":", 1) - result[key.strip()] = parse_value(value) + key = key.strip() + if not key: + raise ValueError(f"Empty key is not allowed in {path} at line {lineno}: {raw_line!r}") + if key in result: + raise ValueError(f"Duplicate key '{key}' in {path} at line {lineno}") + + result[key] = parse_value(value) return result @@ -139,7 +176,11 @@ def load_metadata(path: Path) -> dict: if not isinstance(loaded, dict): raise MetadataLoadError("metadata.yaml must contain a mapping at the top level") return loaded - return _parse_simple_yaml(path) + + try: + return _parse_simple_yaml(path) + except ValueError as exc: + raise MetadataLoadError(str(exc)) from exc def precheck_plugin_directory(plugin_dir: Path) -> dict: @@ -232,20 +273,7 @@ def build_report(results: list[dict]) -> dict: def load_plugins_index(path: Path) -> dict[str, dict]: - data = json.loads(path.read_text(encoding="utf-8")) - if not isinstance(data, dict): - raise ValueError("plugins.json must contain a JSON object") - - for key, value in data.items(): - if not isinstance(key, str): - raise ValueError( - f"plugins.json keys must be strings, got {type(key).__name__!r}: {key!r}" - ) - if not isinstance(value, dict): - raise ValueError( - f"plugins.json values must be objects/dicts; key {key!r} has {type(value).__name__!r}" - ) - return data + return load_plugins_map_file(path, source_name="plugins.json") def combine_requested_names( diff --git a/tests/test_detect_changed_plugins.py b/tests/test_detect_changed_plugins.py index cb807b95..06c027ef 100644 --- a/tests/test_detect_changed_plugins.py +++ b/tests/test_detect_changed_plugins.py @@ -36,6 +36,12 @@ def test_load_plugins_map_returns_dict_for_valid_json(self): self.assertEqual(plugins, {"plugin-a": {"repo": "https://github.com/example/a"}}) + def test_load_plugins_map_rejects_non_dict_entries(self): + module = load_detection_module() + + with self.assertRaisesRegex(ValueError, "plugins.json entry 'plugin-a' on the PR head must be a JSON object"): + module.load_plugins_map('{"plugin-a": "bad"}', source_name="PR head") + class ChangedPluginDetectionTests(unittest.TestCase): def test_detect_changed_plugin_names_returns_only_modified_entries(self): diff --git a/tests/test_validate_plugins.py b/tests/test_validate_plugins.py index b437979f..2e7790e4 100644 --- a/tests/test_validate_plugins.py +++ b/tests/test_validate_plugins.py @@ -203,14 +203,11 @@ def test_parse_simple_yaml_handles_comments_quotes_and_whitespace(self): with tempfile.NamedTemporaryFile("w", suffix=".yml", delete=False) as handle: handle.write( - """ - # leading comment - - key1: value1 # trailing comment - key2: \" spaced value \" - key3: 'another value' - key4: value-with-#-hash - """ + "# leading comment\n\n" + "key1: value1 # trailing comment\n" + 'key2: " spaced value "\n' + "key3: 'another value'\n" + "key4: value-with-#-hash\n" ) metadata_path = Path(handle.name) @@ -224,6 +221,45 @@ def test_parse_simple_yaml_handles_comments_quotes_and_whitespace(self): self.assertEqual(parsed["key3"], "another value") self.assertEqual(parsed["key4"], "value-with-#-hash") + def test_parse_simple_yaml_rejects_indented_lines(self): + module = load_validator_module() + + with tempfile.NamedTemporaryFile("w", suffix=".yml", delete=False) as handle: + handle.write("name: demo\n nested: nope\n") + metadata_path = Path(handle.name) + + try: + with self.assertRaisesRegex(ValueError, "Unsupported YAML indentation"): + module._parse_simple_yaml(metadata_path) + finally: + os.remove(metadata_path) + + def test_parse_simple_yaml_rejects_list_syntax(self): + module = load_validator_module() + + with tempfile.NamedTemporaryFile("w", suffix=".yml", delete=False) as handle: + handle.write("- item\n") + metadata_path = Path(handle.name) + + try: + with self.assertRaisesRegex(ValueError, "Unsupported YAML list syntax"): + module._parse_simple_yaml(metadata_path) + finally: + os.remove(metadata_path) + + def test_parse_simple_yaml_rejects_duplicate_keys(self): + module = load_validator_module() + + with tempfile.NamedTemporaryFile("w", suffix=".yml", delete=False) as handle: + handle.write("name: first\nname: second\n") + metadata_path = Path(handle.name) + + try: + with self.assertRaisesRegex(ValueError, "Duplicate key 'name'"): + module._parse_simple_yaml(metadata_path) + finally: + os.remove(metadata_path) + def test_load_metadata_uses_yaml_safe_load_when_available(self): module = load_validator_module() @@ -268,12 +304,7 @@ def test_load_metadata_uses_simple_parser_when_yaml_unavailable(self): module = load_validator_module() with tempfile.NamedTemporaryFile("w", suffix=".yml", delete=False) as handle: - handle.write( - """ - name: demo-plugin - version: \"0.2.3\" - """ - ) + handle.write('name: demo-plugin\nversion: "0.2.3"\n') metadata_path = Path(handle.name) yaml_backup = getattr(module, "yaml", None) @@ -287,6 +318,22 @@ def test_load_metadata_uses_simple_parser_when_yaml_unavailable(self): self.assertEqual(metadata.get("name"), "demo-plugin") self.assertEqual(metadata.get("version"), "0.2.3") + def test_load_metadata_wraps_fallback_parse_errors(self): + module = load_validator_module() + + with tempfile.NamedTemporaryFile("w", suffix=".yml", delete=False) as handle: + handle.write("name: demo\n nested: nope\n") + metadata_path = Path(handle.name) + + yaml_backup = getattr(module, "yaml", None) + try: + module.yaml = None + with self.assertRaisesRegex(module.MetadataLoadError, "Unsupported YAML indentation"): + module.load_metadata(metadata_path) + finally: + module.yaml = yaml_backup + os.remove(metadata_path) + def test_load_plugins_index_accepts_valid_object(self): module = load_validator_module() @@ -332,7 +379,7 @@ def test_load_plugins_index_rejects_non_dict_values(self): index_path = Path(handle.name) try: - with self.assertRaisesRegex(ValueError, "plugins.json values must be objects/dicts"): + with self.assertRaisesRegex(ValueError, "plugins.json entry 'not-a-dict'.*must be a JSON object"): module.load_plugins_index(index_path) finally: os.remove(index_path) From 312fca109fabc9d984dcad440584606d93459aa8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=82=B9=E6=B0=B8=E8=B5=AB?= <1259085392@qq.com> Date: Fri, 24 Apr 2026 13:24:37 +0900 Subject: [PATCH 09/15] run plugin validation concurrently --- scripts/validate_plugins/run.py | 71 ++++++++++++++++++++++++--------- tests/test_validate_plugins.py | 53 +++++++++++++++++++++++- 2 files changed, 105 insertions(+), 19 deletions(-) diff --git a/scripts/validate_plugins/run.py b/scripts/validate_plugins/run.py index 768bdd96..f81255f8 100644 --- a/scripts/validate_plugins/run.py +++ b/scripts/validate_plugins/run.py @@ -4,6 +4,7 @@ import argparse import asyncio +import concurrent.futures import hashlib import json import os @@ -29,6 +30,7 @@ REQUIRED_METADATA_FIELDS = ("name", "desc", "version", "author") DEFAULT_CLONE_TIMEOUT = 120 +DEFAULT_MAX_WORKERS = 8 CONFLICT_MARKERS = ("<<<<<<<", "=======", ">>>>>>>") @@ -489,29 +491,60 @@ def validate_selected_plugins( work_dir: Path, clone_timeout: int, load_timeout: int, + max_workers: int, ) -> list[dict]: - results = [] total = len(selected) - - for index, (plugin, plugin_data) in enumerate(selected, start=1): - print(f"[{index}/{total}] Validating {plugin}", flush=True) - result = validate_plugin( - plugin=plugin, - plugin_data=plugin_data, - astrbot_path=astrbot_path, - script_path=script_path, - work_dir=work_dir, - clone_timeout=clone_timeout, - load_timeout=load_timeout, + results: list[dict | None] = [None] * total + + def task(index: int, plugin: str, plugin_data: dict) -> tuple[int, dict]: + return ( + index, + validate_plugin( + plugin=plugin, + plugin_data=plugin_data, + astrbot_path=astrbot_path, + script_path=script_path, + work_dir=work_dir, + clone_timeout=clone_timeout, + load_timeout=load_timeout, + ), ) - results.append(result) - status = "PASS" if result.get("ok") else "FAIL" - stage = result.get("stage", "unknown") - message = result.get("message", "") - print(f"[{index}/{total}] {status} {plugin} [{stage}] {message}", flush=True) + with concurrent.futures.ThreadPoolExecutor(max_workers=max_workers) as executor: + future_to_context: dict[concurrent.futures.Future, tuple[int, str]] = {} - return results + for index, (plugin, plugin_data) in enumerate(selected, start=1): + print(f"[{index}/{total}] Queued {plugin}", flush=True) + future = executor.submit(task, index, plugin, plugin_data) + future_to_context[future] = (index, plugin) + + for future in concurrent.futures.as_completed(future_to_context): + index, plugin = future_to_context[future] + try: + original_index, result = future.result() + except Exception as exc: + original_index = index + result = build_result( + plugin=plugin, + repo="", + normalized_repo_url=None, + ok=False, + stage="threadpool", + message=str(exc), + details=traceback.format_exc(), + ) + + results[original_index - 1] = result + status = "PASS" if result.get("ok") else "FAIL" + stage = result.get("stage", "unknown") + message = result.get("message", "") + print(f"[{original_index}/{total}] {status} {plugin} [{stage}] {message}", flush=True) + + finalized = [result for result in results if result is not None] + if len(finalized) != total: + raise RuntimeError("parallel validation finished with missing results") + + return finalized class NullStub: @@ -684,6 +717,7 @@ def build_parser() -> argparse.ArgumentParser: parser.add_argument("--work-dir") parser.add_argument("--clone-timeout", type=int, default=DEFAULT_CLONE_TIMEOUT) parser.add_argument("--load-timeout", type=int, default=300) + parser.add_argument("--max-workers", type=int, default=DEFAULT_MAX_WORKERS) parser.add_argument("--worker", action="store_true") parser.add_argument("--plugin-source-dir") parser.add_argument("--plugin-dir-name") @@ -736,6 +770,7 @@ def main() -> int: work_dir=work_dir, clone_timeout=args.clone_timeout, load_timeout=args.load_timeout, + max_workers=args.max_workers, ) finally: if temp_dir is not None: diff --git a/tests/test_validate_plugins.py b/tests/test_validate_plugins.py index 2e7790e4..39301174 100644 --- a/tests/test_validate_plugins.py +++ b/tests/test_validate_plugins.py @@ -386,6 +386,13 @@ def test_load_plugins_index_rejects_non_dict_values(self): class ValidationProgressTests(unittest.TestCase): + def test_build_parser_defaults_max_workers_to_eight(self): + module = load_validator_module() + + args = module.build_parser().parse_args(["--astrbot-path", "/tmp/AstrBot"]) + + self.assertEqual(args.max_workers, 8) + def test_validate_selected_plugins_emits_progress_and_result_lines(self): module = load_validator_module() selected = [ @@ -406,14 +413,58 @@ def test_validate_selected_plugins_emits_progress_and_result_lines(self): work_dir=Path("/tmp/work"), clone_timeout=60, load_timeout=300, + max_workers=8, ) self.assertEqual(results, fake_results) self.assertEqual(validate_mock.call_count, 2) - print_mock.assert_any_call("[1/2] Validating plugin-a", flush=True) + print_mock.assert_any_call("[1/2] Queued plugin-a", flush=True) print_mock.assert_any_call("[1/2] PASS plugin-a [load] ok", flush=True) print_mock.assert_any_call("[2/2] FAIL plugin-b [metadata] invalid metadata.yaml", flush=True) + def test_validate_selected_plugins_preserves_result_order_with_out_of_order_completion(self): + module = load_validator_module() + selected = [ + ("plugin-a", {"repo": "https://github.com/example/plugin-a"}), + ("plugin-b", {"repo": "https://github.com/example/plugin-b"}), + ("plugin-c", {"repo": "https://github.com/example/plugin-c"}), + ] + futures = [mock.Mock(name="future-a"), mock.Mock(name="future-b"), mock.Mock(name="future-c")] + future_to_result = { + futures[0]: (1, {"plugin": "plugin-a", "ok": True, "stage": "load", "message": "a"}), + futures[1]: (2, {"plugin": "plugin-b", "ok": False, "stage": "metadata", "message": "b"}), + futures[2]: (3, {"plugin": "plugin-c", "ok": True, "stage": "load", "message": "c"}), + } + + executor = mock.MagicMock() + executor.__enter__.return_value = executor + executor.__exit__.return_value = False + executor.submit.side_effect = futures + + def future_result(future): + return future_to_result[future] + + for future in futures: + future.result.side_effect = lambda _timeout=None, future=future: future_result(future) + + with mock.patch.object(module.concurrent.futures, "ThreadPoolExecutor", return_value=executor) as pool_mock: + with mock.patch.object(module.concurrent.futures, "as_completed", return_value=[futures[2], futures[0], futures[1]]): + with mock.patch("builtins.print") as print_mock: + results = module.validate_selected_plugins( + selected=selected, + astrbot_path=Path("/tmp/AstrBot"), + script_path=Path("/tmp/run.py"), + work_dir=Path("/tmp/work"), + clone_timeout=60, + load_timeout=300, + max_workers=8, + ) + + pool_mock.assert_called_once_with(max_workers=8) + self.assertEqual([item["plugin"] for item in results], ["plugin-a", "plugin-b", "plugin-c"]) + print_mock.assert_any_call("[1/3] Queued plugin-a", flush=True) + print_mock.assert_any_call("[3/3] PASS plugin-c [load] c", flush=True) + class ValidatePluginTests(unittest.TestCase): def setUp(self): From 63c827a53da9ba86eb244f294f94aa350e693e62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=82=B9=E6=B0=B8=E8=B5=AB?= <1259085392@qq.com> Date: Fri, 24 Apr 2026 13:41:16 +0900 Subject: [PATCH 10/15] downgrade missing metadata fields to warnings --- scripts/validate_plugins/run.py | 13 ++++++++++--- tests/test_validate_plugins.py | 17 ++++++++++------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/scripts/validate_plugins/run.py b/scripts/validate_plugins/run.py index f81255f8..c4533bb8 100644 --- a/scripts/validate_plugins/run.py +++ b/scripts/validate_plugins/run.py @@ -46,6 +46,7 @@ def build_result( ok: bool, stage: str, message: str, + severity: str | None = None, plugin_dir_name: str | None = None, details: dict | str | None = None, ) -> dict: @@ -56,6 +57,7 @@ def build_result( "ok": ok, "stage": stage, "message": message, + "severity": severity or ("pass" if ok else "fail"), } if plugin_dir_name: result["plugin_dir_name"] = plugin_dir_name @@ -212,6 +214,7 @@ def precheck_plugin_directory(plugin_dir: Path) -> dict: if missing: return { "ok": False, + "severity": "warn", "stage": "metadata", "message": f"missing required metadata fields: {', '.join(missing)}", } @@ -262,12 +265,14 @@ def build_worker_sys_path(*, astrbot_root: Path, astrbot_path: Path) -> list[str def build_report(results: list[dict]) -> dict: - passed = sum(1 for result in results if result.get("ok")) - failed = len(results) - passed + passed = sum(1 for result in results if result.get("severity") == "pass") + warned = sum(1 for result in results if result.get("severity") == "warn") + failed = sum(1 for result in results if result.get("severity") == "fail") return { "summary": { "total": len(results), "passed": passed, + "warned": warned, "failed": failed, }, "results": results, @@ -442,6 +447,7 @@ def validate_plugin( ok=False, stage=precheck["stage"], message=precheck["message"], + severity=precheck.get("severity"), details=precheck.get("details"), ) @@ -535,7 +541,8 @@ def task(index: int, plugin: str, plugin_data: dict) -> tuple[int, dict]: ) results[original_index - 1] = result - status = "PASS" if result.get("ok") else "FAIL" + severity = result.get("severity", "pass" if result.get("ok") else "fail") + status = {"pass": "PASS", "warn": "WARN", "fail": "FAIL"}.get(severity, "FAIL") stage = result.get("stage", "unknown") message = result.get("message", "") print(f"[{original_index}/{total}] {status} {plugin} [{stage}] {message}", flush=True) diff --git a/tests/test_validate_plugins.py b/tests/test_validate_plugins.py index 39301174..8eaa4e7e 100644 --- a/tests/test_validate_plugins.py +++ b/tests/test_validate_plugins.py @@ -400,8 +400,8 @@ def test_validate_selected_plugins_emits_progress_and_result_lines(self): ("plugin-b", {"repo": "https://github.com/example/plugin-b"}), ] fake_results = [ - {"plugin": "plugin-a", "ok": True, "stage": "load", "message": "ok"}, - {"plugin": "plugin-b", "ok": False, "stage": "metadata", "message": "invalid metadata.yaml"}, + {"plugin": "plugin-a", "ok": True, "severity": "pass", "stage": "load", "message": "ok"}, + {"plugin": "plugin-b", "ok": False, "severity": "warn", "stage": "metadata", "message": "missing required metadata fields: desc"}, ] with mock.patch.object(module, "validate_plugin", side_effect=fake_results) as validate_mock: @@ -420,7 +420,7 @@ def test_validate_selected_plugins_emits_progress_and_result_lines(self): self.assertEqual(validate_mock.call_count, 2) print_mock.assert_any_call("[1/2] Queued plugin-a", flush=True) print_mock.assert_any_call("[1/2] PASS plugin-a [load] ok", flush=True) - print_mock.assert_any_call("[2/2] FAIL plugin-b [metadata] invalid metadata.yaml", flush=True) + print_mock.assert_any_call("[2/2] WARN plugin-b [metadata] missing required metadata fields: desc", flush=True) def test_validate_selected_plugins_preserves_result_order_with_out_of_order_completion(self): module = load_validator_module() @@ -592,6 +592,7 @@ def test_reports_missing_required_metadata_fields(self): result = module.precheck_plugin_directory(plugin_dir) self.assertFalse(result["ok"]) + self.assertEqual(result["severity"], "warn") self.assertEqual(result["stage"], "metadata") self.assertIn("desc", result["message"]) self.assertIn("version", result["message"]) @@ -652,19 +653,21 @@ def test_worker_sys_path_includes_astrbot_root_before_codebase(self): class ReportBuilderTests(unittest.TestCase): - def test_build_report_counts_passed_and_failed_results(self): + def test_build_report_counts_passed_warned_and_failed_results(self): module = load_validator_module() report = module.build_report( [ - {"plugin": "plugin-a", "ok": True, "stage": "load", "message": "ok"}, - {"plugin": "plugin-b", "ok": False, "stage": "metadata", "message": "missing desc"}, + {"plugin": "plugin-a", "ok": True, "severity": "pass", "stage": "load", "message": "ok"}, + {"plugin": "plugin-b", "ok": False, "severity": "warn", "stage": "metadata", "message": "missing desc"}, + {"plugin": "plugin-c", "ok": False, "severity": "fail", "stage": "load", "message": "boom"}, ] ) - self.assertEqual(report["summary"]["total"], 2) + self.assertEqual(report["summary"]["total"], 3) self.assertEqual(report["summary"]["passed"], 1) self.assertEqual(report["summary"]["failed"], 1) + self.assertEqual(report["summary"]["warned"], 1) self.assertEqual(report["results"][1]["plugin"], "plugin-b") From 2901a3426d4d3ef5d55bb9722ae25c492f15ea44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=82=B9=E6=B0=B8=E8=B5=AB?= <1259085392@qq.com> Date: Fri, 24 Apr 2026 13:51:17 +0900 Subject: [PATCH 11/15] add workflow concurrency input --- .github/workflows/validate-plugin-smoke.yml | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/.github/workflows/validate-plugin-smoke.yml b/.github/workflows/validate-plugin-smoke.yml index 6884b589..7df52590 100644 --- a/.github/workflows/validate-plugin-smoke.yml +++ b/.github/workflows/validate-plugin-smoke.yml @@ -6,6 +6,8 @@ on: - "plugins.json" - "scripts/validate_plugins/**" - ".github/workflows/validate-plugin-smoke.yml" + schedule: + - cron: "0 2 * * *" workflow_dispatch: inputs: plugin_names: @@ -20,6 +22,10 @@ on: description: "AstrBot git ref to validate against" required: false default: "master" + max_workers: + description: "Maximum concurrent plugin validations" + required: false + default: "8" jobs: validate-plugin-smoke: @@ -37,7 +43,18 @@ jobs: echo "ASTRBOT_REF=${{ inputs.astrbot_ref }}" >> "$GITHUB_ENV" echo "PLUGIN_NAME_LIST=${{ inputs.plugin_names }}" >> "$GITHUB_ENV" echo "PLUGIN_LIMIT=${{ inputs.plugin_limit }}" >> "$GITHUB_ENV" + echo "MAX_WORKERS=${{ inputs.max_workers }}" >> "$GITHUB_ENV" + echo "SHOULD_VALIDATE=true" >> "$GITHUB_ENV" + + - name: Set scheduled validation inputs + if: github.event_name == 'schedule' + run: | + echo "ASTRBOT_REF=master" >> "$GITHUB_ENV" + echo "PLUGIN_NAME_LIST=" >> "$GITHUB_ENV" + echo "PLUGIN_LIMIT=" >> "$GITHUB_ENV" + echo "MAX_WORKERS=8" >> "$GITHUB_ENV" echo "SHOULD_VALIDATE=true" >> "$GITHUB_ENV" + echo "VALIDATION_NOTE=Running scheduled full plugin validation." >> "$GITHUB_ENV" - name: Detect changed plugins from pull request if: github.event_name == 'pull_request' @@ -86,6 +103,10 @@ jobs: args+=(--limit "$PLUGIN_LIMIT") fi + if [ -n "${MAX_WORKERS:-}" ]; then + args+=(--max-workers "$MAX_WORKERS") + fi + python scripts/validate_plugins/run.py "${args[@]}" - name: Upload validation report From 5525e276b08e1aa8e8b4ddb7efb05c07167062bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=82=B9=E6=B0=B8=E8=B5=AB?= <1259085392@qq.com> Date: Fri, 24 Apr 2026 14:01:42 +0900 Subject: [PATCH 12/15] handle base ref fetch fallback --- .../validate_plugins/detect_changed_plugins.py | 3 +-- tests/test_detect_changed_plugins.py | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/scripts/validate_plugins/detect_changed_plugins.py b/scripts/validate_plugins/detect_changed_plugins.py index 3e8584b3..b78b5d79 100644 --- a/scripts/validate_plugins/detect_changed_plugins.py +++ b/scripts/validate_plugins/detect_changed_plugins.py @@ -55,9 +55,8 @@ def resolve_astrbot_ref() -> str: def detect_pull_request_selection(*, repo_root: Path, base_ref: str) -> dict[str, object]: - fetch_base_ref(base_ref) - try: + fetch_base_ref(base_ref) base = load_plugins_map(read_base_plugins_json(base_ref), source_name=f"base ref {base_ref}") except (subprocess.CalledProcessError, ValueError): base = {} diff --git a/tests/test_detect_changed_plugins.py b/tests/test_detect_changed_plugins.py index 06c027ef..64678af7 100644 --- a/tests/test_detect_changed_plugins.py +++ b/tests/test_detect_changed_plugins.py @@ -74,6 +74,24 @@ def test_resolve_astrbot_ref_falls_back_to_master(self): class PullRequestDetectionTests(unittest.TestCase): + def test_detect_pull_request_selection_handles_fetch_base_ref_failure(self): + module = load_detection_module() + + with tempfile.TemporaryDirectory() as tmp_dir: + repo_root = Path(tmp_dir) + plugins_json = repo_root / "plugins.json" + plugins_json.write_text('{"plugin-a": {"repo": "https://github.com/example/a"}}', encoding="utf-8") + + with mock.patch.object( + module, + "fetch_base_ref", + side_effect=module.subprocess.CalledProcessError(1, ["git", "fetch"]), + ): + result = module.detect_pull_request_selection(repo_root=repo_root, base_ref="main") + + self.assertEqual(result["changed"], ["plugin-a"]) + self.assertEqual(result["validation_note"], "") + def test_detect_pull_request_selection_handles_missing_base_file(self): module = load_detection_module() From 3b8e255605ffd58f2a0ed58666d46406806d0224 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=82=B9=E6=B0=B8=E8=B5=AB?= <1259085392@qq.com> Date: Fri, 24 Apr 2026 14:14:51 +0900 Subject: [PATCH 13/15] validate plugin directory names safely --- scripts/validate_plugins/run.py | 24 ++++++++++++++++++-- tests/test_validate_plugins.py | 39 +++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/scripts/validate_plugins/run.py b/scripts/validate_plugins/run.py index c4533bb8..3a33c75d 100644 --- a/scripts/validate_plugins/run.py +++ b/scripts/validate_plugins/run.py @@ -219,7 +219,16 @@ def precheck_plugin_directory(plugin_dir: Path) -> dict: "message": f"missing required metadata fields: {', '.join(missing)}", } - plugin_name = metadata["name"].strip() + try: + plugin_name = validate_plugin_dir_name(metadata["name"]) + except ValueError as exc: + return { + "ok": False, + "stage": "metadata", + "message": "invalid plugin directory name", + "details": str(exc), + } + entry_candidates = [plugin_dir / "main.py", plugin_dir / f"{plugin_name}.py"] if not any(path.exists() for path in entry_candidates): return { @@ -298,6 +307,17 @@ def sanitize_name(name: str) -> str: return sanitized or "plugin" +def validate_plugin_dir_name(name: str) -> str: + candidate = name.strip() + if not candidate or candidate in {".", ".."}: + raise ValueError("unsafe plugin_dir_name") + if "/" in candidate or "\\" in candidate: + raise ValueError("unsafe plugin_dir_name") + if ".." in candidate: + raise ValueError("unsafe plugin_dir_name") + return candidate + + def build_plugin_clone_dir(work_dir: Path, plugin: str) -> Path: digest = hashlib.sha256(plugin.encode("utf-8")).hexdigest()[:8] return work_dir / f"{sanitize_name(plugin)}-{digest}" @@ -662,7 +682,7 @@ async def run_worker_load_check(plugin_dir_name: str, normalized_repo_url: str) normalized_repo_url=normalized_repo_url, ok=False, stage="load", - message=error or "plugin load failed", + message=str(error) if error else "plugin load failed", plugin_dir_name=plugin_dir_name, details=manager.failed_plugin_dict.get(plugin_dir_name), ) diff --git a/tests/test_validate_plugins.py b/tests/test_validate_plugins.py index 8eaa4e7e..933f7bf8 100644 --- a/tests/test_validate_plugins.py +++ b/tests/test_validate_plugins.py @@ -615,6 +615,24 @@ def test_reports_invalid_metadata_yaml_without_raising(self): self.assertIn("invalid metadata.yaml", result["message"]) self.assertIn("could not find expected ':'", result["details"]) + def test_rejects_unsafe_plugin_dir_name_from_metadata(self): + module = load_validator_module() + + with tempfile.TemporaryDirectory() as tmp_dir: + plugin_dir = Path(tmp_dir) + (plugin_dir / "metadata.yaml").write_text( + "name: ../escape\ndesc: demo\nversion: 1.0.0\nauthor: AstrBot Team\n", + encoding="utf-8", + ) + (plugin_dir / "main.py").write_text("print('hello')\n", encoding="utf-8") + + result = module.precheck_plugin_directory(plugin_dir) + + self.assertFalse(result["ok"]) + self.assertEqual(result["stage"], "metadata") + self.assertEqual(result["message"], "invalid plugin directory name") + self.assertIn("unsafe plugin_dir_name", result["details"]) + class WorkerCommandTests(unittest.TestCase): def test_build_worker_command_contains_required_arguments(self): @@ -652,6 +670,27 @@ def test_worker_sys_path_includes_astrbot_root_before_codebase(self): ) +class WorkerLoadCheckTests(unittest.IsolatedAsyncioTestCase): + async def test_stringifies_non_string_plugin_load_error_message(self): + module = load_validator_module() + + class FakeManager: + def __init__(self, context, config): + del context, config + self.failed_plugin_dict = {"demo_plugin": {"error": "detail"}} + + async def load(self, specified_dir_name: str): + del specified_dir_name + return False, {"reason": "boom"} + + with mock.patch.dict(sys.modules, {"astrbot.core.star.star_manager": mock.Mock(PluginManager=FakeManager)}): + result = await module.run_worker_load_check("demo_plugin", "https://github.com/example/demo") + + self.assertFalse(result["ok"]) + self.assertEqual(result["stage"], "load") + self.assertEqual(result["message"], "{'reason': 'boom'}") + + class ReportBuilderTests(unittest.TestCase): def test_build_report_counts_passed_warned_and_failed_results(self): module = load_validator_module() From 95aa286dd9e3772f80751f1af009357792eec951 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=82=B9=E6=B0=B8=E8=B5=AB?= <1259085392@qq.com> Date: Fri, 24 Apr 2026 14:16:35 +0900 Subject: [PATCH 14/15] upgrade actions for node 24 --- .github/workflows/validate-plugin-smoke.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/validate-plugin-smoke.yml b/.github/workflows/validate-plugin-smoke.yml index 7df52590..b398271b 100644 --- a/.github/workflows/validate-plugin-smoke.yml +++ b/.github/workflows/validate-plugin-smoke.yml @@ -33,7 +33,7 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@v4 + uses: actions/checkout@v6 with: fetch-depth: 0 @@ -71,7 +71,7 @@ jobs: - name: Set up Python if: env.SHOULD_VALIDATE == 'true' - uses: actions/setup-python@v5 + uses: actions/setup-python@v6 with: python-version: "3.12" @@ -111,7 +111,7 @@ jobs: - name: Upload validation report if: always() - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: validation-report path: validation-report.json From 65556b0965618c608ca90f7e702b26a0046cc054 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=82=B9=E6=B0=B8=E8=B5=AB?= <1259085392@qq.com> Date: Fri, 24 Apr 2026 14:34:00 +0900 Subject: [PATCH 15/15] tighten validator CLI and warning semantics --- scripts/validate_plugins/run.py | 19 ++++++++--- tests/test_validate_plugins.py | 57 +++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 5 deletions(-) diff --git a/scripts/validate_plugins/run.py b/scripts/validate_plugins/run.py index 3a33c75d..941f5c2b 100644 --- a/scripts/validate_plugins/run.py +++ b/scripts/validate_plugins/run.py @@ -38,6 +38,13 @@ class MetadataLoadError(ValueError): pass +def positive_int(raw_value: str) -> int: + value = int(raw_value) + if value <= 0: + raise argparse.ArgumentTypeError("must be a positive integer") + return value + + def build_result( *, plugin: str, @@ -50,14 +57,16 @@ def build_result( plugin_dir_name: str | None = None, details: dict | str | None = None, ) -> dict: + resolved_severity = severity or ("pass" if ok else "fail") + resolved_ok = True if resolved_severity in {"pass", "warn"} else ok result = { "plugin": plugin, "repo": repo, "normalized_repo_url": normalized_repo_url, - "ok": ok, + "ok": resolved_ok, "stage": stage, "message": message, - "severity": severity or ("pass" if ok else "fail"), + "severity": resolved_severity, } if plugin_dir_name: result["plugin_dir_name"] = plugin_dir_name @@ -742,9 +751,9 @@ def build_parser() -> argparse.ArgumentParser: parser.add_argument("--astrbot-path") parser.add_argument("--report-path", default="validation-report.json") parser.add_argument("--work-dir") - parser.add_argument("--clone-timeout", type=int, default=DEFAULT_CLONE_TIMEOUT) - parser.add_argument("--load-timeout", type=int, default=300) - parser.add_argument("--max-workers", type=int, default=DEFAULT_MAX_WORKERS) + parser.add_argument("--clone-timeout", type=positive_int, default=DEFAULT_CLONE_TIMEOUT) + parser.add_argument("--load-timeout", type=positive_int, default=300) + parser.add_argument("--max-workers", type=positive_int, default=DEFAULT_MAX_WORKERS) parser.add_argument("--worker", action="store_true") parser.add_argument("--plugin-source-dir") parser.add_argument("--plugin-dir-name") diff --git a/tests/test_validate_plugins.py b/tests/test_validate_plugins.py index 933f7bf8..e0f0699e 100644 --- a/tests/test_validate_plugins.py +++ b/tests/test_validate_plugins.py @@ -393,6 +393,18 @@ def test_build_parser_defaults_max_workers_to_eight(self): self.assertEqual(args.max_workers, 8) + def test_build_parser_rejects_non_positive_worker_and_timeout_values(self): + module = load_validator_module() + + with self.assertRaises(SystemExit): + module.build_parser().parse_args(["--astrbot-path", "/tmp/AstrBot", "--max-workers", "0"]) + + with self.assertRaises(SystemExit): + module.build_parser().parse_args(["--astrbot-path", "/tmp/AstrBot", "--clone-timeout", "0"]) + + with self.assertRaises(SystemExit): + module.build_parser().parse_args(["--astrbot-path", "/tmp/AstrBot", "--load-timeout", "0"]) + def test_validate_selected_plugins_emits_progress_and_result_lines(self): module = load_validator_module() selected = [ @@ -546,6 +558,51 @@ def test_precheck_failure_is_mapped_into_result(self): self.assertEqual(result["message"], "invalid metadata") self.assertEqual(result["details"], "line 3") + def test_precheck_warning_is_non_fatal_in_final_result(self): + with mock.patch.object(self.module, "clone_plugin_repo"): + with mock.patch.object( + self.module, + "precheck_plugin_directory", + return_value={ + "ok": False, + "severity": "warn", + "stage": "metadata", + "message": "missing required metadata fields: desc", + }, + ): + result = self.call_validate_plugin() + + self.assertTrue(result["ok"]) + self.assertEqual(result["severity"], "warn") + self.assertEqual(result["stage"], "metadata") + + def test_load_timeout_uses_process_output_details(self): + timeout = subprocess.TimeoutExpired( + cmd=[sys.executable, str(self.script_path)], + timeout=60, + output="timeout-stdout", + stderr="timeout-stderr", + ) + + with mock.patch.object( + self.module, + "precheck_plugin_directory", + return_value={"ok": True, "plugin_dir_name": "demo-plugin", "message": "ok", "stage": "precheck"}, + ): + with mock.patch.object(self.module, "clone_plugin_repo"): + with mock.patch.object(subprocess, "run", side_effect=timeout): + with mock.patch.object( + self.module, + "build_process_output_details", + return_value={"stdout": "timeout-stdout", "stderr": "timeout-stderr"}, + ) as details_mock: + result = self.call_validate_plugin() + + self.assertEqual(result["stage"], "timeout") + self.assertEqual(result["plugin_dir_name"], "demo-plugin") + self.assertEqual(result["details"], {"stdout": "timeout-stdout", "stderr": "timeout-stderr"}) + details_mock.assert_called_once_with(stdout="timeout-stdout", stderr="timeout-stderr") + def test_successful_clone_and_precheck_invokes_worker_and_parses_output(self): completed = subprocess.CompletedProcess( args=["python3", "run.py"],