From 8622fe40f58e2a69875e2a1526de12688de73543 Mon Sep 17 00:00:00 2001 From: sickn33 Date: Mon, 30 Mar 2026 21:43:29 +0200 Subject: [PATCH] fix(marketplace): Publish plugin sync atomically --- tools/scripts/sync_editorial_bundles.py | 131 ++++++++++++------ tools/scripts/tests/test_editorial_bundles.py | 26 ++++ 2 files changed, 115 insertions(+), 42 deletions(-) diff --git a/tools/scripts/sync_editorial_bundles.py b/tools/scripts/sync_editorial_bundles.py index d1f04cce..af02400b 100644 --- a/tools/scripts/sync_editorial_bundles.py +++ b/tools/scripts/sync_editorial_bundles.py @@ -4,11 +4,14 @@ from __future__ import annotations import argparse import errno import json +import os import re import shutil +import tempfile import time +import uuid from pathlib import Path -from typing import Any +from typing import Any, Callable from _project_paths import find_repo_root from plugin_compatibility import build_report as build_plugin_compatibility_report @@ -587,16 +590,55 @@ def _remove_tree(path: Path, retries: int = 3, delay_seconds: float = 0.1) -> No def _materialize_plugin_skills(root: Path, destination_root: Path, skill_ids: list[str]) -> None: - if destination_root.is_symlink() or destination_root.is_file(): - destination_root.unlink() - elif destination_root.exists(): - _remove_tree(destination_root) destination_root.mkdir(parents=True, exist_ok=True) for skill_id in skill_ids: _copy_skill_directory(root, skill_id, destination_root) +def _remove_path(path: Path) -> None: + if path.is_symlink() or path.is_file(): + path.unlink() + return + if path.exists(): + _remove_tree(path) + + +def _replace_directory_atomically( + destination_root: Path, + populate: Callable[[Path], None], +) -> None: + parent = destination_root.parent + parent.mkdir(parents=True, exist_ok=True) + + staging_root = Path( + tempfile.mkdtemp( + prefix=f".{destination_root.name}.staging-", + dir=parent, + ) + ) + backup_root = parent / f".{destination_root.name}.backup-{uuid.uuid4().hex}" + replaced_existing = False + + try: + populate(staging_root) + + if destination_root.exists() or destination_root.is_symlink(): + os.replace(destination_root, backup_root) + replaced_existing = True + + os.replace(staging_root, destination_root) + except Exception: + if replaced_existing and backup_root.exists() and not destination_root.exists(): + os.replace(backup_root, destination_root) + raise + finally: + if staging_root.exists(): + _remove_path(staging_root) + if backup_root.exists(): + _remove_path(backup_root) + + def _supported_skill_ids( compatibility: dict[str, dict[str, Any]], target: str, @@ -619,18 +661,24 @@ def _sync_root_plugins( codex_root = root / "plugins" / ROOT_CODEX_PLUGIN_NAME claude_root = root / "plugins" / ROOT_CLAUDE_PLUGIN_DIRNAME - _materialize_plugin_skills(root, codex_root / "skills", codex_skill_ids) - _materialize_plugin_skills(root, claude_root / "skills", claude_skill_ids) + def populate_codex_root(staging_root: Path) -> None: + _materialize_plugin_skills(root, staging_root / "skills", codex_skill_ids) + _write_json( + staging_root / ".codex-plugin" / "plugin.json", + _root_codex_plugin_manifest(metadata, len(codex_skill_ids)), + ) + + def populate_claude_root(staging_root: Path) -> None: + _materialize_plugin_skills(root, staging_root / "skills", claude_skill_ids) + _write_json( + staging_root / ".claude-plugin" / "plugin.json", + _root_claude_plugin_manifest(metadata, len(claude_skill_ids)), + ) + + _replace_directory_atomically(codex_root, populate_codex_root) + _replace_directory_atomically(claude_root, populate_claude_root) - _write_json( - codex_root / ".codex-plugin" / "plugin.json", - _root_codex_plugin_manifest(metadata, len(codex_skill_ids)), - ) claude_manifest = _root_claude_plugin_manifest(metadata, len(claude_skill_ids)) - _write_json( - claude_root / ".claude-plugin" / "plugin.json", - claude_manifest, - ) _write_json(root / CLAUDE_PLUGIN_PATH, claude_manifest) @@ -645,25 +693,26 @@ def _sync_bundle_plugin_directory( plugin_name = _bundle_plugin_name(bundle["id"]) plugin_root = root / "plugins" / plugin_name - if plugin_root.exists(): - _remove_tree(plugin_root) - bundle_skills_root = plugin_root / "skills" - bundle_skills_root.mkdir(parents=True, exist_ok=True) + def populate_bundle_plugin(staging_root: Path) -> None: + bundle_skills_root = staging_root / "skills" + bundle_skills_root.mkdir(parents=True, exist_ok=True) - for skill in bundle["skills"]: - _copy_skill_directory(root, skill["id"], bundle_skills_root) + for skill in bundle["skills"]: + _copy_skill_directory(root, skill["id"], bundle_skills_root) - if support["claude"]: - _write_json( - plugin_root / ".claude-plugin" / "plugin.json", - _bundle_claude_plugin_manifest(metadata, bundle), - ) - if support["codex"]: - _write_json( - plugin_root / ".codex-plugin" / "plugin.json", - _bundle_codex_plugin_manifest(metadata, bundle), - ) + if support["claude"]: + _write_json( + staging_root / ".claude-plugin" / "plugin.json", + _bundle_claude_plugin_manifest(metadata, bundle), + ) + if support["codex"]: + _write_json( + staging_root / ".codex-plugin" / "plugin.json", + _bundle_codex_plugin_manifest(metadata, bundle), + ) + + _replace_directory_atomically(plugin_root, populate_bundle_plugin) def sync_editorial_bundle_plugins( @@ -673,13 +722,18 @@ def sync_editorial_bundle_plugins( bundle_support: dict[str, dict[str, Any]], ) -> None: plugins_root = root / "plugins" - for candidate in plugins_root.glob("antigravity-bundle-*"): - if candidate.is_dir(): - shutil.rmtree(candidate) - + expected_plugin_names = { + _bundle_plugin_name(bundle["id"]) + for bundle in bundles + if bundle_support[bundle["id"]]["codex"] or bundle_support[bundle["id"]]["claude"] + } for bundle in bundles: _sync_bundle_plugin_directory(root, metadata, bundle, bundle_support[bundle["id"]]) + for candidate in plugins_root.glob("antigravity-bundle-*"): + if candidate.is_dir() and candidate.name not in expected_plugin_names: + _remove_tree(candidate) + def load_editorial_bundles(root: Path) -> list[dict[str, Any]]: root = Path(root) @@ -710,13 +764,6 @@ def sync_editorial_bundles(root: Path) -> None: root / CODEX_MARKETPLACE_PATH, _render_codex_marketplace(bundles, bundle_support), ) - _write_json( - root / CODEX_ROOT_PLUGIN_PATH, - _root_codex_plugin_manifest( - metadata, - len(_supported_skill_ids(compatibility, "codex")), - ), - ) sync_editorial_bundle_plugins(root, metadata, bundles, bundle_support) diff --git a/tools/scripts/tests/test_editorial_bundles.py b/tools/scripts/tests/test_editorial_bundles.py index c30089a3..e16ae535 100644 --- a/tools/scripts/tests/test_editorial_bundles.py +++ b/tools/scripts/tests/test_editorial_bundles.py @@ -2,6 +2,7 @@ import importlib.util import errno import pathlib import sys +import tempfile from unittest import mock import unittest @@ -160,6 +161,31 @@ class EditorialBundlesTests(unittest.TestCase): self.assertEqual(calls["count"], 2) sleep_mock.assert_called_once() + def test_replace_directory_atomically_swaps_only_after_staging_is_ready(self): + with tempfile.TemporaryDirectory() as temp_dir: + temp_root = pathlib.Path(temp_dir) + destination = temp_root / "plugin" + old_file = destination / "skills" / "old.txt" + old_file.parent.mkdir(parents=True, exist_ok=True) + old_file.write_text("old", encoding="utf-8") + + observed = {} + + def populate(staging_root): + new_file = staging_root / "skills" / "new.txt" + new_file.parent.mkdir(parents=True, exist_ok=True) + new_file.write_text("new", encoding="utf-8") + + observed["old_visible_during_populate"] = old_file.is_file() + observed["new_hidden_during_populate"] = not (destination / "skills" / "new.txt").exists() + + editorial_bundles._replace_directory_atomically(destination, populate) + + self.assertTrue(observed["old_visible_during_populate"]) + self.assertTrue(observed["new_hidden_during_populate"]) + self.assertFalse(old_file.exists()) + self.assertTrue((destination / "skills" / "new.txt").is_file()) + if __name__ == "__main__": unittest.main()