From c1ee8ba51eb69c153b4ca9d530aefe47f49e27ea Mon Sep 17 00:00:00 2001 From: onatozmenn Date: Fri, 18 Sep 2026 15:30:05 +0300 Subject: [PATCH 1/3] fix(iterate): ship real vendor manifest dirs for Codex install Co-authored-by: openhands --- scripts/sync_extensions.py | 37 ++++++-- skills/iterate/.claude-plugin | 1 - skills/iterate/.claude-plugin/plugin.json | 13 +++ skills/iterate/.codex-plugin | 1 - skills/iterate/.codex-plugin/plugin.json | 13 +++ tests/test_sync_extensions.py | 101 ++++++++++++++++++++++ 6 files changed, 159 insertions(+), 7 deletions(-) delete mode 120000 skills/iterate/.claude-plugin create mode 100644 skills/iterate/.claude-plugin/plugin.json delete mode 120000 skills/iterate/.codex-plugin create mode 100644 skills/iterate/.codex-plugin/plugin.json diff --git a/scripts/sync_extensions.py b/scripts/sync_extensions.py index 040ff241..4e3b6c63 100644 --- a/scripts/sync_extensions.py +++ b/scripts/sync_extensions.py @@ -9,7 +9,8 @@ 3. **coverage** — warn when a skill/plugin directory is not listed in any marketplace, or a marketplace entry points to a missing directory. 4. **symlinks** — enforce ``.plugin/`` as the canonical manifest directory - with vendor symlinks (``.claude-plugin``, ``.codex-plugin``). + with vendor manifests (``.claude-plugin``, ``.codex-plugin``), each + either a symlink or a real directory mirroring ``plugin.json``. Usage: python scripts/sync_extensions.py # run all, write changes @@ -375,13 +376,30 @@ def sync_coverage(*, check: bool) -> list[str]: return problems -# ── 4. Vendor symlinks ──────────────────────────────────────────────── +# ── 4. Vendor manifests ───────────────────────────────────────────────── VENDOR_SYMLINKS = [".claude-plugin", ".codex-plugin"] # add new vendors here +def _vendor_manifest_matches(directory: Path, vendor: str) -> bool: + """True when a real vendor directory mirrors the canonical manifest.""" + src = directory / ".plugin" / "plugin.json" + dst = directory / vendor / "plugin.json" + return ( + src.is_file() + and dst.is_file() + and src.read_bytes() == dst.read_bytes() + ) + + def _check_vendor_symlinks(directory: Path, check: bool) -> list[str]: - """Check/fix vendor symlinks for a single directory with .plugin/.""" + """Check/fix vendor manifests for a single directory with .plugin/. + + A vendor path is either a symlink to ``.plugin/`` or a real directory + whose ``plugin.json`` mirrors the canonical one. Real directories exist + because some installers (e.g. Codex ``plugin add``) drop symlinked + directories instead of copying them; see skills/iterate. + """ problems: list[str] = [] canon = directory / ".plugin" if not canon.is_dir(): @@ -393,6 +411,15 @@ def _check_vendor_symlinks(directory: Path, check: bool) -> list[str]: if target == canon.resolve(): continue problems.append(f"wrong target: {link.relative_to(REPO_ROOT)} → {link.readlink()}") + elif link.is_dir(): + if _vendor_manifest_matches(directory, vendor): + continue + problems.append(f"stale manifest copy: {link.relative_to(REPO_ROOT)}") + if not check: + (link / "plugin.json").write_bytes( + (canon / "plugin.json").read_bytes() + ) + continue elif link.exists(): problems.append(f"not a symlink: {link.relative_to(REPO_ROOT)}") continue @@ -405,11 +432,11 @@ def _check_vendor_symlinks(directory: Path, check: bool) -> list[str]: def sync_symlinks(*, check: bool) -> list[str]: - """Ensure every directory with .plugin/ also has vendor symlinks. + """Ensure every directory with .plugin/ has discoverable vendor manifests. Scans both plugins/ and skills/ directories. Skills that ship a ``.plugin/`` manifest (e.g. those with ``commands/``) need vendor - symlinks so that Codex and Claude Code can discover them. + manifests so that Codex and Claude Code can discover them. """ problems: list[str] = [] for base in SKILL_DIRS: diff --git a/skills/iterate/.claude-plugin b/skills/iterate/.claude-plugin deleted file mode 120000 index 665797f0..00000000 --- a/skills/iterate/.claude-plugin +++ /dev/null @@ -1 +0,0 @@ -.plugin \ No newline at end of file diff --git a/skills/iterate/.claude-plugin/plugin.json b/skills/iterate/.claude-plugin/plugin.json new file mode 100644 index 00000000..9981af94 --- /dev/null +++ b/skills/iterate/.claude-plugin/plugin.json @@ -0,0 +1,13 @@ +{ + "name": "iterate", + "version": "1.0.0", + "description": "Iterate on a GitHub pull request — drive it through CI, code review, and QA until it is merge-ready.", + "author": { + "name": "OpenHands", + "email": "contact@all-hands.dev" + }, + "homepage": "https://github.com/OpenHands/extensions", + "repository": "https://github.com/OpenHands/extensions", + "license": "MIT", + "keywords": ["github", "ci", "review", "qa", "pull-request", "iterate"] +} diff --git a/skills/iterate/.codex-plugin b/skills/iterate/.codex-plugin deleted file mode 120000 index 665797f0..00000000 --- a/skills/iterate/.codex-plugin +++ /dev/null @@ -1 +0,0 @@ -.plugin \ No newline at end of file diff --git a/skills/iterate/.codex-plugin/plugin.json b/skills/iterate/.codex-plugin/plugin.json new file mode 100644 index 00000000..9981af94 --- /dev/null +++ b/skills/iterate/.codex-plugin/plugin.json @@ -0,0 +1,13 @@ +{ + "name": "iterate", + "version": "1.0.0", + "description": "Iterate on a GitHub pull request — drive it through CI, code review, and QA until it is merge-ready.", + "author": { + "name": "OpenHands", + "email": "contact@all-hands.dev" + }, + "homepage": "https://github.com/OpenHands/extensions", + "repository": "https://github.com/OpenHands/extensions", + "license": "MIT", + "keywords": ["github", "ci", "review", "qa", "pull-request", "iterate"] +} diff --git a/tests/test_sync_extensions.py b/tests/test_sync_extensions.py index 8dfb703d..ed973fd2 100644 --- a/tests/test_sync_extensions.py +++ b/tests/test_sync_extensions.py @@ -1,5 +1,6 @@ """Tests for scripts/sync_extensions.py core functions.""" +import shutil import sys from pathlib import Path @@ -18,6 +19,7 @@ parse_frontmatter, slash_triggers, sync_commands, + sync_symlinks, ) @@ -288,6 +290,105 @@ def test_manually_edited_file_detected_in_check_mode(self, tmp_path, monkeypatch assert any("manually-edited" in p for p in problems) +# ── vendor manifests ───────────────────────────────────────────────── + +def _make_plugin_skill(root: Path, name: str = "demo") -> Path: + """Create a fake skill dir with a canonical .plugin/plugin.json.""" + skill = root / "skills" / name + (skill / ".plugin").mkdir(parents=True) + (skill / ".plugin" / "plugin.json").write_text('{"name": "demo"}\n') + return skill + + +def _make_real_vendor_mirror(skill: Path, vendor: str = ".codex-plugin") -> Path: + path = skill / vendor + path.mkdir(exist_ok=True) + (path / "plugin.json").write_text('{"name": "demo"}\n') + return path + + +def _point_sync_at(tmp_path, monkeypatch): + monkeypatch.setattr("sync_extensions.SKILL_DIRS", [tmp_path / "skills"]) + monkeypatch.setattr("sync_extensions.REPO_ROOT", tmp_path) + + +class TestVendorManifests: + def test_iterate_vendor_dirs_are_real_and_mirror_canonical(self): + """Codex drops symlinked dirs on install, so iterate ships real ones.""" + iterate = REPO_ROOT / "skills" / "iterate" + canon = (iterate / ".plugin" / "plugin.json").read_bytes() + for vendor in (".codex-plugin", ".claude-plugin"): + path = iterate / vendor + assert not path.is_symlink(), f"{vendor} must not be a symlink" + assert path.is_dir(), f"{vendor} must be a real directory" + assert (path / "plugin.json").read_bytes() == canon + + def test_symlink_to_canonical_passes(self, tmp_path, monkeypatch): + skill = _make_plugin_skill(tmp_path) + try: + for vendor in (".codex-plugin", ".claude-plugin"): + (skill / vendor).symlink_to(".plugin", target_is_directory=True) + except OSError: + pytest.skip("symlinks need privileges on this platform") + _point_sync_at(tmp_path, monkeypatch) + + assert sync_symlinks(check=True) == [] + + def test_real_dir_mirror_passes_and_stale_copy_is_flagged(self, tmp_path, monkeypatch): + skill = _make_plugin_skill(tmp_path) + vendor = _make_real_vendor_mirror(skill) + _make_real_vendor_mirror(skill, ".claude-plugin") + _point_sync_at(tmp_path, monkeypatch) + + assert sync_symlinks(check=True) == [] + + (vendor / "plugin.json").write_text('{"name": "stale"}\n') + problems = sync_symlinks(check=True) + assert any("stale manifest copy" in p for p in problems) + + def test_fix_mode_refreshes_stale_copy(self, tmp_path, monkeypatch): + skill = _make_plugin_skill(tmp_path) + vendor = _make_real_vendor_mirror(skill) + _make_real_vendor_mirror(skill, ".claude-plugin") + (vendor / "plugin.json").write_text('{"name": "stale"}\n') + _point_sync_at(tmp_path, monkeypatch) + + problems = sync_symlinks(check=False) + assert any("stale manifest copy" in p for p in problems) + assert (vendor / "plugin.json").read_text() == '{"name": "demo"}\n' + assert sync_symlinks(check=True) == [] + + def test_lossy_install_copy_keeps_codex_manifest(self, tmp_path): + """Mimic Codex `plugin add`, which drops symlinked directories. + + The installed cache must still contain + `.codex-plugin/plugin.json` for the plugin to load. + """ + src = REPO_ROOT / "skills" / "iterate" + dst = tmp_path / "iterate" + shutil.copytree( + src, + dst, + ignore=lambda d, names: [ + n for n in names if (Path(d) / n).is_symlink() + ], + ) + manifest = dst / ".codex-plugin" / "plugin.json" + assert manifest.is_file(), "Codex install lost .codex-plugin/plugin.json" + assert manifest.read_bytes() == (src / ".plugin" / "plugin.json").read_bytes() + + def test_missing_vendor_still_gets_symlink(self, tmp_path, monkeypatch): + skill = _make_plugin_skill(tmp_path) + _point_sync_at(tmp_path, monkeypatch) + try: + assert sync_symlinks(check=False) == [] + except OSError: + pytest.skip("symlinks need privileges on this platform") + + assert (skill / ".codex-plugin").is_symlink() + assert (skill / ".claude-plugin").is_symlink() + + # ── marketplace source paths ───────────────────────────────────────── class TestMarketplaceSourcePaths: From 6d63d68ff63340ba12b36db7a8ff651da5814512 Mon Sep 17 00:00:00 2001 From: onatozmenn Date: Fri, 18 Sep 2026 17:44:10 +0300 Subject: [PATCH 2/3] test: accept real vendor manifest dirs in plugin loading checks Co-authored-by: openhands --- tests/test_skill_plugin_loading.py | 40 +++++++++++++++++++----------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/tests/test_skill_plugin_loading.py b/tests/test_skill_plugin_loading.py index 250f7e59..66fc44eb 100644 --- a/tests/test_skill_plugin_loading.py +++ b/tests/test_skill_plugin_loading.py @@ -1,8 +1,10 @@ """Test that skills listed in marketplaces can be loaded as Codex/Claude plugins. Every marketplace entry that references a ``skills/`` directory needs a -``.plugin/plugin.json`` manifest and vendor symlinks (``.codex-plugin``, +``.plugin/plugin.json`` manifest and vendor manifests (``.codex-plugin``, ``.claude-plugin``) so that Codex and Claude Code can discover and load them. +A vendor manifest is either a symlink to ``.plugin/`` or a real directory +mirroring ``plugin.json`` (Codex install drops symlinked dirs; see issue #257). Regression test for: https://github.com/OpenHands/extensions/issues/201 """ @@ -61,18 +63,17 @@ def test_all_marketplace_skills_have_plugin_json(self): f"{', '.join(missing)}" ) - def test_all_marketplace_skills_have_vendor_symlinks(self): - """Every marketplace skill with a manifest must have vendor symlinks.""" + def test_all_marketplace_skills_have_vendor_manifests(self): + """Every marketplace skill with a manifest must have vendor manifests.""" problems = [] for name, path in _marketplace_skill_entries(): if not (path / ".plugin" / "plugin.json").exists(): continue for vendor in VENDOR_SYMLINKS: - link = path / vendor - if not link.is_symlink(): + if not _vendor_manifest_ok(path, vendor): problems.append(f"{name}/{vendor}") assert not problems, ( - f"Missing vendor symlinks: {', '.join(problems)}" + f"Missing vendor manifests: {', '.join(problems)}" ) def test_all_manifests_have_required_fields(self): @@ -127,8 +128,20 @@ def test_iterate_loads_as_sdk_plugin(self): assert "verify" in command_names +def _vendor_manifest_ok(directory: Path, vendor: str) -> bool: + """A vendor manifest is a symlink to .plugin/ or a real dir mirror.""" + link = directory / vendor + if link.is_symlink(): + return link.resolve() == (directory / ".plugin").resolve() + if link.is_dir(): + src = directory / ".plugin" / "plugin.json" + dst = link / "plugin.json" + return src.is_file() and dst.is_file() and src.read_bytes() == dst.read_bytes() + return False + + class TestVendorSymlinksForManifests: - """Every directory with .plugin/ must have vendor symlinks.""" + """Every directory with .plugin/ must have vendor manifests.""" @pytest.fixture( params=list(_all_dirs_with_plugin_manifest()), @@ -137,13 +150,10 @@ class TestVendorSymlinksForManifests: def dir_with_manifest(self, request): return request.param - def test_has_vendor_symlinks(self, dir_with_manifest): - """Directories with .plugin/ must have .claude-plugin and .codex-plugin symlinks.""" + def test_has_vendor_manifests(self, dir_with_manifest): + """Directories with .plugin/ must have .claude-plugin and .codex-plugin manifests.""" for vendor in VENDOR_SYMLINKS: - link = dir_with_manifest / vendor - assert link.is_symlink(), ( - f"{link.relative_to(REPO_ROOT)} must be a symlink to .plugin" - ) - assert link.resolve() == (dir_with_manifest / ".plugin").resolve(), ( - f"{link.relative_to(REPO_ROOT)} must point to .plugin" + assert _vendor_manifest_ok(dir_with_manifest, vendor), ( + f"{dir_with_manifest / vendor} must be a symlink to .plugin " + "or a real directory mirroring plugin.json" ) From 7d657fb3c85cc3fd9c986ee26fea847fc0e08795 Mon Sep 17 00:00:00 2001 From: onatozmenn Date: Wed, 23 Sep 2026 11:32:29 +0300 Subject: [PATCH 3/3] \fix(tests): make missing-vendor test pass on Linux" --- tests/test_sync_extensions.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_sync_extensions.py b/tests/test_sync_extensions.py index ed973fd2..33d02696 100644 --- a/tests/test_sync_extensions.py +++ b/tests/test_sync_extensions.py @@ -381,12 +381,13 @@ def test_missing_vendor_still_gets_symlink(self, tmp_path, monkeypatch): skill = _make_plugin_skill(tmp_path) _point_sync_at(tmp_path, monkeypatch) try: - assert sync_symlinks(check=False) == [] + sync_symlinks(check=False) except OSError: pytest.skip("symlinks need privileges on this platform") assert (skill / ".codex-plugin").is_symlink() assert (skill / ".claude-plugin").is_symlink() + assert sync_symlinks(check=True) == [] # ── marketplace source paths ─────────────────────────────────────────