fix(iterate): ship real vendor manifest dirs for Codex install

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:

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"]
+}

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"]
+}

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"
             )

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,106 @@ 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:
+            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 ─────────────────────────────────────────
 
 class TestMarketplaceSourcePaths: