From ae4833e2611ff8a8ae4b1a98a1ae966865d41bc8 Mon Sep 17 00:00:00 2001 From: Aqr-K <95741669+Aqr-K@users.noreply.github.com> Date: Tue, 27 Oct 2026 05:43:16 +0800 Subject: [PATCH] fix(publish): pin plugin reads and writes to one commit (#7204) --- docs/testing.md | 6 + skills/publish-moviepilot-plugin/SKILL.md | 4 + .../scripts/publish_plugin.py | 50 ++++--- tests/test_plugin_publish_snapshot.py | 137 ++++++++++++++++++ tests/test_publish_moviepilot_plugin_skill.py | 15 +- 5 files changed, 192 insertions(+), 20 deletions(-) create mode 100644 tests/test_plugin_publish_snapshot.py diff --git a/docs/testing.md b/docs/testing.md index e0ff2d68f..2619a85fa 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -329,6 +329,12 @@ def test_recognize_prefers_explicit_identity(sample_meta, monkeypatch): - **没有直接 `assert` 不等于空测试**：`pytest.raises`、辅助断言、迁移 round-trip、导入兼容和“不抛异常”都是有效契约；只有名称或注释声称存在的覆盖则需要进一步核实。 - **删除后验证**：运行受影响文件，核对保留用例及生产代码行/分支覆盖；跨领域清理再运行 `python tests/run.py`。不降低覆盖率基线、不新增 skip、不削弱架构或兼容门禁。 +## 插件发布快照与并发回归 + +`tests/test_plugin_publish_snapshot.py` 固定读取快照，并在 package 返回后确定性推进远端引用， +断言非强推冲突保留并发作者更新；自动初始化新仓后必须重新固定、读取和合并，不能把新主分支 +当空仓库。测试只替换 GitHub I/O，保留真实文件读取、清单合并与 Git Data 请求构造。 + ## 排查测试问题 - **收集报错（collection error）**：多为 import 期副作用或顶层桩污染。优先改成真实 import（conftest 已隔离临时库，真实 `settings`/helper 可加载）+ 方法内 patch，而不是靠事后还原（收集期污染发生在 import 那一刻，事后还原太晚）。 diff --git a/skills/publish-moviepilot-plugin/SKILL.md b/skills/publish-moviepilot-plugin/SKILL.md index 084710370..1e140d597 100644 --- a/skills/publish-moviepilot-plugin/SKILL.md +++ b/skills/publish-moviepilot-plugin/SKILL.md @@ -169,6 +169,10 @@ zip/tar.gz + sha256 资产）： - For Vue federation plugins, publish built runtime assets under `dist/assets/` when they are present; do not exclude them as generated files. - Do not overwrite or remove package entries for other plugins. +- Pin the remote commit before reading the package or plugin files. Build the + single publish commit from that same tree and parent. If the target branch + advances meanwhile, stop on the non-fast-forward conflict, then preview again + against a fresh snapshot; never force-push or reuse stale merged content. - Do not log or print GitHub token values. - For push operations, report created, updated, deleted, skipped, and rejected files separately. diff --git a/skills/publish-moviepilot-plugin/scripts/publish_plugin.py b/skills/publish-moviepilot-plugin/scripts/publish_plugin.py index 4662f9944..1854aef6e 100644 --- a/skills/publish-moviepilot-plugin/scripts/publish_plugin.py +++ b/skills/publish-moviepilot-plugin/scripts/publish_plugin.py @@ -279,18 +279,19 @@ class GitHubClient: except json.JSONDecodeError: return raw.decode("utf-8", errors="replace") - def get_file(self, path: str) -> Optional[FileState]: + def get_file(self, path: str, ref: Optional[str] = None) -> Optional[FileState]: """ 读取远端单个文件，文件不存在时返回 None。 :param path: 仓库内 POSIX 路径 + :param ref: 固定读取的提交；未指定时读取目标分支 :return: 文件状态或 None """ encoded_path = quote_path(path) try: data = self.request( "GET", - f"/repos/{self.repo}/contents/{encoded_path}?ref={quote(self.branch)}", + f"/repos/{self.repo}/contents/{encoded_path}?ref={quote(ref or self.branch)}", ) except GitHubError as err: if err.status == 404: @@ -302,17 +303,18 @@ class GitHubClient: content = base64.b64decode(raw_content.encode("utf-8"), validate=False) return FileState(path=path, content=content, sha=data.get("sha")) - def list_files(self, prefix: str) -> dict[str, FileState]: + def list_files(self, prefix: str, ref: Optional[str] = None) -> dict[str, FileState]: """ 递归列出远端路径下的全部文件。 :param prefix: 仓库内目录路径 + :param ref: 固定读取的提交；递归列表和文件内容共用此引用 :return: 以仓库路径为键的远端文件状态 """ normalized_prefix = normalize_remote_path(prefix) if not normalized_prefix: return {} - tree = self.get_tree() + tree = self.get_tree(ref=ref) files: dict[str, FileState] = {} prefix_with_slash = f"{normalized_prefix}/" for item in tree: @@ -322,22 +324,25 @@ class GitHubClient: if path == normalized_prefix or path.startswith(prefix_with_slash): files[path] = FileState(path=path, content=b"", sha=item.get("sha")) for path in list(files): - file_state = self.get_file(path) + file_state = self.get_file(path, ref=ref) if file_state is not None: files[path] = file_state return files - def get_tree(self) -> list[dict[str, Any]]: + def get_tree(self, ref: Optional[str] = None) -> list[dict[str, Any]]: """ 读取目标分支的递归 Git tree。 + :param ref: 固定读取的提交；未指定时查询目标分支 :return: Git tree 条目列表 """ - branch_data = self.request( - "GET", - f"/repos/{self.repo}/branches/{quote(self.branch)}", - ) - commit_sha = branch_data.get("commit", {}).get("sha") + commit_sha = ref + if not commit_sha: + branch_data = self.request( + "GET", + f"/repos/{self.repo}/branches/{quote(self.branch)}", + ) + commit_sha = branch_data.get("commit", {}).get("sha") if not commit_sha: raise GitHubError(0, f"无法读取分支 {self.branch} 的提交 SHA") tree_data = self.request( @@ -457,6 +462,7 @@ class GitHubClient: files: dict[str, bytes], deletes: list[str], message: str, + base_commit: Optional[dict[str, Any]], ) -> Optional[str]: """ 用 Git Data API 把多个文件的变更合并成一次提交。 @@ -468,11 +474,12 @@ class GitHubClient: :param files: 仓库路径到文件内容的映射 :param deletes: 需要删除的仓库路径 :param message: 提交说明 + :param base_commit: 文件读取前固定的基线提交；None 表示基线为空仓库 :return: 新提交 SHA；没有任何变更时返回 None """ if not files and not deletes: return None - head = self.get_branch_commit() + head = base_commit base_tree = None parents: list[str] = [] if head: @@ -1113,6 +1120,7 @@ def load_remote_files_for_push( :return: 远端插件文件、远端 package 文件、仓库是否已存在 """ client: GitHubClient = context["client"] + context["base_commit"] = None repo_exists = client.repo_exists() if not repo_exists and not args.create_repo_if_missing: raise GitHubError( @@ -1121,9 +1129,15 @@ def load_remote_files_for_push( ) if not repo_exists: return {}, None, False + # 清单、插件内容、删除候选与提交父树必须属于同一个不可变快照。 + head = client.get_branch_commit() + context["base_commit"] = head + if head is None: + return {}, None, True + ref = str(head["sha"]) return ( - client.list_files(context["remote_prefix"]), - client.get_file(context["layout"].package_file), + client.list_files(context["remote_prefix"], ref=ref), + client.get_file(context["layout"].package_file, ref=ref), True, ) @@ -1174,6 +1188,10 @@ def push(args: argparse.Namespace) -> dict[str, Any]: ) entry = read_package_entry(context["local_repo"], context["layout"], context["plugin_id"]) remote_files, remote_package, repo_existed = load_remote_files_for_push(context, args) + if not repo_existed and not args.dry_run: + client.create_repo(private=args.private) + # auto_init 会创建默认分支；创建后重新固定快照，并重新读取、比较及合并全部内容。 + remote_files, remote_package, _ = load_remote_files_for_push(context, args) package_content = merge_package_content( remote_package.content if remote_package else None, context["plugin_id"], @@ -1192,8 +1210,6 @@ def push(args: argparse.Namespace) -> dict[str, Any]: payload["dry_run"] = True return payload - if not repo_existed: - client.create_repo(private=args.private) message = args.message or f"Publish {context['plugin_id']}" applied: dict[str, list[str]] = {"create": [], "update": [], "delete": []} files_to_write: dict[str, bytes] = {} @@ -1211,7 +1227,7 @@ def push(args: argparse.Namespace) -> dict[str, Any]: if remote_files[path].sha: delete_paths.append(path) applied["delete"].append(path) - commit_sha = client.commit_files(files_to_write, delete_paths, message) + commit_sha = client.commit_files(files_to_write, delete_paths, message, context["base_commit"]) payload["applied"] = applied payload["commit_sha"] = commit_sha payload["message"] = "插件已以单个提交推送到 GitHub 仓库。" diff --git a/tests/test_plugin_publish_snapshot.py b/tests/test_plugin_publish_snapshot.py new file mode 100644 index 000000000..984df56ea --- /dev/null +++ b/tests/test_plugin_publish_snapshot.py @@ -0,0 +1,137 @@ +""" +验证插件发布读取固定远端快照及新建仓、并发提交的保护。 +""" + + + +import base64 +import importlib.util +import json +import sys +from argparse import Namespace +from pathlib import Path +from urllib.parse import parse_qs, urlsplit + +import pytest + + +@pytest.fixture +def publisher(monkeypatch): + """ + 加载真实发布脚本，并由夹具恢复动态模块登记。 + """ + script = Path(__file__).resolve().parents[1] / "skills/publish-moviepilot-plugin/scripts/publish_plugin.py" + spec = importlib.util.spec_from_file_location("review_publisher", script) + module = importlib.util.module_from_spec(spec) + monkeypatch.setitem(sys.modules, spec.name, module) + spec.loader.exec_module(module) + return module + + +@pytest.mark.parametrize("created", [False, True]) +@pytest.mark.parametrize("concurrent", [False, True]) +def test_publish_uses_one_snapshot_and_preserves_concurrent_package(tmp_path, monkeypatch, publisher, concurrent, created): + """ + 完整发布入口须以同一读取快照建树；并发推进时拒绝覆盖其他作者更新。 + """ + module = publisher + packages = {"head-0": {"OtherPlugin": {"version": "1.0.0"}}, + "head-1": {"OtherPlugin": {"version": "2.0.0"}}} + state = {"head": "head-0", "blobs": {}, "tree": {}, "commit": {}, "read_refs": [], "exists": not created} + plugin_path = "plugins.v3/myplugin/__init__.py" + remote_plugin = b"old plugin\n" + + class BoundaryClient(module.GitHubClient): + """ + 只替换 GitHub I/O，保留真实文件读取、合并和 Git Data 请求构造。 + """ + + def __init__(self): + """ + 配置隔离仓库身份，不初始化真实网络客户端。 + """ + self.repo = "fixture/repo" + self.branch = "main" + + def repo_exists(self): + """ + 创建前返回缺失，创建后返回存在。 + """ + return state["exists"] + + def request(self, method, path, payload=None, accept=""): + """ + 在清单读取完成时推进分支，并按 Git 非强推语义拒绝旧父提交。 + """ + del accept + if method == "GET" and path == "/user": + return {"login": "fixture"} + if method == "POST" and path == "/user/repos": + assert payload["auto_init"] is True + # 自动初始化创建主分支；其后首次读取前，另一个作者也可提交插件清单。 + state["exists"] = True + return {"default_branch": "main"} + if method == "POST" and path.endswith("/git/refs"): + raise module.GitHubError(422, "Reference already exists") + if method == "GET" and "/branches/" in path: + return {"commit": {"sha": state["head"], "commit": {"tree": {"sha": "tree-" + state["head"]}}}} + if method == "GET" and "/git/trees/" in path: + ref = path.split("/git/trees/")[1].split("?")[0] + state["read_refs"].append(ref) + return {"tree": [{"path": plugin_path, "type": "blob", "sha": "plugin-old"}]} + if method == "GET" and "/contents/" in path: + url = urlsplit(path) + ref = parse_qs(url.query)["ref"][0] + ref = state["head"] if ref == "main" else ref + state["read_refs"].append(ref) + content_path = url.path.split("/contents/")[1] + if content_path == "package.v3.json": + content = json.dumps(packages[ref]).encode() + if concurrent: + state["head"] = "head-1" + else: + assert content_path == plugin_path + content = remote_plugin + return {"type": "file", "sha": "old", "content": base64.b64encode(content).decode()} + if method == "POST" and path.endswith("/git/blobs"): + sha = f"blob-{len(state['blobs'])}" + state["blobs"][sha] = base64.b64decode(payload["content"]) + return {"sha": sha} + if method == "POST" and path.endswith("/git/trees"): + state["tree"] = payload + return {"sha": "new-tree"} + if method == "POST" and path.endswith("/git/commits"): + state["commit"] = payload + return {"sha": "new-head"} + if method == "PATCH" and "/git/refs/heads/" in path: + assert payload["force"] is False + if state["commit"]["parents"] != [state["head"]]: + raise module.GitHubError(422, "Update is not a fast forward") + for entry in state["tree"]["tree"]: + if entry["path"] == "package.v3.json": + packages["new-head"] = json.loads(state["blobs"][entry["sha"]]) + state["head"] = "new-head" + return {} + raise AssertionError((method, path)) + + (tmp_path / "plugins.v3/myplugin").mkdir(parents=True) + (tmp_path / plugin_path).write_bytes(b"new plugin\n") + (tmp_path / "package.v3.json").write_text(json.dumps({"MyPlugin": {"version": "1.1.0"}})) + client = BoundaryClient() + context = {"repo": client.repo, "branch": client.branch, "plugin_id": "MyPlugin", "local_repo": tmp_path, + "layout": module.Layout("package.v3.json", "plugins.v3"), "remote_prefix": "plugins.v3/myplugin", + "client": client, "excludes": list(module.DEFAULT_EXCLUDES), "includes": []} + monkeypatch.setattr(module, "build_context", lambda *_args, **_kwargs: context) + args = Namespace(dry_run=False, delete_remote=False, create_repo_if_missing=created, private=False, message="Publish") + if concurrent: + with pytest.raises(module.GitHubError, match="fast forward"): + module.push(args) + assert state["head"] == "head-1" + assert packages[state["head"]]["OtherPlugin"]["version"] == "2.0.0" + else: + result = module.push(args) + assert result["commit_sha"] == "new-head" + assert packages["new-head"] == {"OtherPlugin": {"version": "1.0.0"}, "MyPlugin": {"version": "1.1.0"}} + assert state["read_refs"] == ["head-0", "head-0", "head-0"] + assert state["tree"]["base_tree"] == "tree-head-0" + assert state["commit"]["parents"] == ["head-0"] diff --git a/tests/test_publish_moviepilot_plugin_skill.py b/tests/test_publish_moviepilot_plugin_skill.py index cde1b242c..5430243b7 100644 --- a/tests/test_publish_moviepilot_plugin_skill.py +++ b/tests/test_publish_moviepilot_plugin_skill.py @@ -110,9 +110,15 @@ def test_push_creates_public_repo_when_explicitly_requested(tmp_path: Path) -> N def repo_exists(self) -> bool: """ - 返回仓库不存在。 + 创建前仓库不存在，创建后可读取新分支状态。 """ - return False + return self.created_private is not None + + def get_branch_commit(self) -> None: + """ + 模拟建仓后目标分支尚未初始化。 + """ + return None def create_repo(self, private: bool = False) -> dict[str, Any]: """ @@ -126,10 +132,12 @@ def test_push_creates_public_repo_when_explicitly_requested(tmp_path: Path) -> N files: dict[str, bytes], deletes: list[str], message: str, + base_commit: dict[str, Any] | None, ) -> str: """ 记录一次批量提交的文件集合。 """ + assert base_commit is None self.commit_count += 1 self.committed_files = sorted(files) self.committed_deletes = list(deletes) @@ -325,6 +333,7 @@ def test_commit_files_creates_single_commit_for_all_changes() -> None: {"package.v3.json": b"{}", "plugins.v3/demo/__init__.py": b""}, ["plugins.v3/demo/old.py"], "Publish Demo", + client.get_branch_commit(), ) assert commit_sha == "new-commit" @@ -368,7 +377,7 @@ def test_commit_files_creates_branch_in_empty_repo() -> None: raise AssertionError(f"未预期的请求: {method} {path}") client.request = fake_request # type: ignore[method-assign] - commit_sha = client.commit_files({"package.v3.json": b"{}"}, [], "Init") + commit_sha = client.commit_files({"package.v3.json": b"{}"}, [], "Init", client.get_branch_commit()) assert commit_sha == "root-commit" commit_calls = [payload for method, path, payload in calls if path.endswith("/git/commits")]