From 953bd9d2504ac8ee38ef93ee86dbb5d1caaec793 Mon Sep 17 00:00:00 2001 From: minasarustamyan <156230623+minasarustamyan@users.noreply.github.com> Date: Wed, 29 Apr 2026 19:25:57 +0200 Subject: [PATCH] fix(marketplace): use plugin.json name in synth marketplace.json (#133) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the /plugin UI 'Plugin not found in marketplace' bug. Synth marketplace.json catalog 'name' now reads from /.claude-plugin/plugin.json (with fallback to upstream marketplace.json name). On-disk plugins/-/ layout preserved so cross-marketplace files don't collide. /marketplace/info exposes both name and prefixed_name (BREAKING — downstream tooling parsing 'name' for the slug-prefixed form must switch to prefixed_name). --- CHANGELOG.md | 8 +++ CLAUDE.md | 15 +++++- app/marketplace_server/git_backend.py | 8 ++- app/marketplace_server/packager.py | 15 ++++-- src/marketplace_filter.py | 71 +++++++++++++++++++++++--- tests/test_marketplace_filter.py | 73 +++++++++++++++++++++++++++ tests/test_marketplace_server_git.py | 43 +++++++++++++++- tests/test_marketplace_server_zip.py | 57 +++++++++++++++++++-- 8 files changed, 269 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 209ac01..8a6ea9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,14 @@ CalVer image tags (`stable-YYYY.MM.N`, `dev-YYYY.MM.N`) are produced for every C ## [Unreleased] +### Changed + +- **BREAKING** `GET /marketplace/info` (admin-only debug endpoint) `name` field now returns the plugin's authoritative name from its `plugin.json` (e.g. `plug-x`) instead of the slug-prefixed form (`-`). The slug-prefixed form moved to a new `prefixed_name` field next to it; `original_name` is unchanged. Side-effect of the `/plugin` UI fix below — the synth marketplace.json's `name` field had to switch over for Claude Code's catalog lookup to work, and `/marketplace/info` mirrors that surface for consistency. Any downstream tooling that parsed the `name` field expecting the slug-prefixed format must now read `prefixed_name`. + +### Fixed + +- **`/plugin` UI in Claude Code rendered "Plugin not found in marketplace" in the Components panel** for every plugin Agnes served, even though agents/skills/commands loaded correctly under the plugin's own namespace. Root cause: the synthetic `.claude-plugin/marketplace.json` listed each plugin under a slug-prefixed `name` (`-`) while the plugin's authoritative `.claude-plugin/plugin.json` kept the original name. Claude Code resolves the loaded plugin back to its catalog entry by `plugin.json` name, so the lookup missed every entry. The synth manifest now reads the plugin's authoritative name from `/.claude-plugin/plugin.json` (falling back to the upstream marketplace.json's `name` when the plugin manifest is absent or unreadable). The directory layout under `plugins/-/...` keeps the prefix so two upstream marketplaces that ship a same-named plugin still get distinct on-disk paths in the ZIP / git tree — their catalog entries will then collide under the same `name`, which is the correct surface (admin RBAC decides which upstream wins, same as if a user added both upstream marketplaces directly to Claude Code). `/marketplace/info` now exposes `prefixed_name` alongside `name` so operators can still disambiguate cross-marketplace shadowing. + ## [0.18.0] — 2026-04-29 ### Added diff --git a/CLAUDE.md b/CLAUDE.md index 9be3f18..80a14a7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -312,8 +312,19 @@ joins `resource_grants ↔ marketplace_plugins` (matching caller's `user_group_members`. Admin is treated as a regular group here — no god-mode shortcut for the marketplace feed, so admins curate their own view by granting plugins to the Admin group (or any group they belong to). -Plugin names are prefixed with marketplace slug (`-`) so two -marketplaces with the same plugin name don't collide in the aggregated view. +On-disk layout in the served ZIP / git tree uses a slug-prefixed directory +(`plugins/-/`) so two marketplaces shipping a same-named +plugin don't overwrite each other's files. The synth marketplace.json's +`name` field, however, is the plugin's authoritative name from its own +`.claude-plugin/plugin.json` (with a fallback to the upstream +marketplace.json `name`) — Claude Code's `/plugin` UI resolves a loaded +plugin back to its catalog entry by `plugin.json` name, so the catalog +entry's `name` must match. Same-named plugins from two upstream +marketplaces therefore collide in the catalog by design; admin RBAC +(which grants survive the filter) decides which one wins, identical to +how Claude Code behaves when a user adds two upstream marketplaces with +overlapping plugin names directly. `/marketplace/info` exposes both +`name` and `prefixed_name` so operators can disambiguate. Cache: content-addressed bare repos at `${DATA_DIR}/marketplaces/git-cache/` keyed by sha256(filtered content). Two users with the same RBAC view share diff --git a/app/marketplace_server/git_backend.py b/app/marketplace_server/git_backend.py index c3fa533..46afd97 100644 --- a/app/marketplace_server/git_backend.py +++ b/app/marketplace_server/git_backend.py @@ -53,11 +53,15 @@ def cache_dir() -> Path: def _merged_manifest_bytes(plugins: list[dict], etag: str) -> bytes: """Same manifest as the ZIP channel produces — kept inline to avoid - importing packager internals into the hot path.""" + importing packager internals into the hot path. + + See packager._merged_manifest for the rationale on `name` = + manifest_name vs. `source` = prefixed_name. + """ entries = [] for plugin in plugins: entry = dict(plugin["raw"]) - entry["name"] = plugin["prefixed_name"] + entry["name"] = plugin["manifest_name"] entry["source"] = f"./plugins/{plugin['prefixed_name']}" if plugin.get("version") and "version" not in entry: entry["version"] = plugin["version"] diff --git a/app/marketplace_server/packager.py b/app/marketplace_server/packager.py index 4fa82a4..abb817c 100644 --- a/app/marketplace_server/packager.py +++ b/app/marketplace_server/packager.py @@ -63,8 +63,14 @@ def _merged_manifest(plugins: List[dict], etag: str) -> Dict[str, Any]: """Synthesize .claude-plugin/marketplace.json over the filtered plugin set. Each entry copies the plugin's cached `raw` manifest, then overrides: - - `name` = prefixed_name - - `source` = "./plugins/" (flat relative path in the ZIP) + - `name` = manifest_name (from the plugin's own plugin.json — must + match the loaded plugin's identity, or the + `/plugin` UI Components panel can't link + the loaded plugin back to its catalog + entry; see src.marketplace_filter) + - `source` = "./plugins/" (slug-prefixed dir avoids + cross-marketplace file collisions in the + flat ZIP / git tree layout) All other fields (version, description, author, homepage, keywords, ...) are preserved so Claude Code UI looks the same as if the user pulled from the upstream marketplace directly. @@ -72,7 +78,7 @@ def _merged_manifest(plugins: List[dict], etag: str) -> Dict[str, Any]: entries: List[dict] = [] for plugin in plugins: entry = dict(plugin["raw"]) # shallow copy — we only override two keys - entry["name"] = plugin["prefixed_name"] + entry["name"] = plugin["manifest_name"] entry["source"] = f"./plugins/{plugin['prefixed_name']}" # Always honor the cached version on the aggregated manifest — the # plugin_dir on disk might have drifted if sync fetched a new commit @@ -108,8 +114,9 @@ def build_info(conn: duckdb.DuckDBPyConnection, user: dict) -> Dict[str, Any]: "plugin_count": len(plugins), "plugins": [ { - "name": p["prefixed_name"], + "name": p["manifest_name"], "original_name": p["original_name"], + "prefixed_name": p["prefixed_name"], "marketplace_slug": p["marketplace_slug"], "version": p.get("version"), "description": p["raw"].get("description"), diff --git a/src/marketplace_filter.py b/src/marketplace_filter.py index ea3d3d7..3c3afb5 100644 --- a/src/marketplace_filter.py +++ b/src/marketplace_filter.py @@ -8,10 +8,24 @@ is no implicit Everyone membership and no god-mode shortcut for the marketplace feed — admins curate their own view by granting plugins to a group they belong to (Admin or otherwise). -Plugins from different marketplaces that happen to share a name are NOT the -same plugin — the caller needs both. We therefore prefix every plugin name -with its marketplace slug (`-`) when projecting out, so -the merged marketplace.json never has colliding entries. +Two distinct identifiers travel through the resolver: + +- ``prefixed_name`` (``-``) drives the on-disk directory + layout in the served ZIP / git tree (``plugins//...``) so + two marketplaces shipping a same-named plugin don't overwrite each other's + files. +- ``manifest_name`` (read from the plugin's own + ``.claude-plugin/plugin.json`` ``name`` field, with a fallback to the + upstream marketplace.json ``name``) is what the synth marketplace.json's + ``name`` field uses. Claude Code's ``/plugin`` UI resolves a loaded plugin + back to its catalog entry by ``plugin.json`` ``name``, so the catalog + entry must match — anything else and the Components panel renders + "Plugin not found in marketplace". + +Same-named plugins from two upstream marketplaces therefore collide in the +served catalog by design; admin RBAC (which grants survive the filter) +decides which one wins, identical to Claude Code's behavior when a user +adds two upstream marketplaces with overlapping plugin names directly. resource_id format for ``marketplace_plugin`` grants is ``/`` — the slash is the canonical separator; @@ -51,6 +65,36 @@ def _prefixed_name(slug: str, plugin_name: str) -> str: return f"{slug}-{plugin_name}" +def _resolve_manifest_name(plugin_dir: Path, fallback: str) -> str: + """Return the plugin's authoritative `name` from its `.claude-plugin/plugin.json`. + + Claude Code resolves a loaded plugin back to its marketplace catalog + entry by the name declared in the plugin's own `plugin.json`. The synth + `marketplace.json` we serve must use that same name, otherwise the + `/plugin` UI Components panel can't link the loaded plugin to its + catalog entry and renders "Plugin not found in marketplace". + + Falls back to ``fallback`` (the upstream marketplace.json's plugin name) + when plugin.json is missing, unreadable, has no string `name`, or has + an empty/whitespace-only `name` — same defensive style as + ``src.marketplace.read_plugins``: never crash, always return a usable + value. + """ + pj = plugin_dir / ".claude-plugin" / "plugin.json" + if not pj.is_file(): + return fallback + try: + data = json.loads(pj.read_text(encoding="utf-8")) + except (OSError, ValueError): + return fallback + if not isinstance(data, dict): + return fallback + name = data.get("name") + if isinstance(name, str) and name.strip(): + return name.strip() + return fallback + + def resolve_allowed_plugins( conn: duckdb.DuckDBPyConnection, user: dict ) -> List[dict]: @@ -60,8 +104,19 @@ def resolve_allowed_plugins( { "marketplace_id": str, # also the slug (they are the same) "marketplace_slug": str, - "original_name": str, - "prefixed_name": str, # "-" + "original_name": str, # name from upstream marketplace.json + "prefixed_name": str, # "-" — drives + # the on-disk dir layout in the ZIP / + # git tree (cross-marketplace files + # don't collide). + "manifest_name": str, # name from the plugin's own + # .claude-plugin/plugin.json (or + # original_name fallback) — drives + # the `name` field in the synth + # marketplace.json we serve, so the + # Claude Code UI's catalog lookup + # matches the loaded plugin's + # namespace. "version": str | None, "raw": dict, # parsed marketplace.json plugin entry "plugin_dir": Path, # ${DATA_DIR}/marketplaces//plugins/ @@ -99,15 +154,17 @@ def resolve_allowed_plugins( result: List[dict] = [] for marketplace_id, name, version, raw in rows: slug = marketplace_id # registry.id IS the slug (see src/marketplace.py) + plugin_dir = root / slug / "plugins" / name result.append( { "marketplace_id": marketplace_id, "marketplace_slug": slug, "original_name": name, "prefixed_name": _prefixed_name(slug, name), + "manifest_name": _resolve_manifest_name(plugin_dir, fallback=name), "version": version, "raw": _resolve_raw(raw), - "plugin_dir": root / slug / "plugins" / name, + "plugin_dir": plugin_dir, } ) return result diff --git a/tests/test_marketplace_filter.py b/tests/test_marketplace_filter.py index e5a14c4..20e1afa 100644 --- a/tests/test_marketplace_filter.py +++ b/tests/test_marketplace_filter.py @@ -176,6 +176,79 @@ class TestResolveAllowedPlugins: assert result == [] +def _seed_grant_and_user( + conn, *, slug: str, plugin: str, user_id: str = "u-mn" +) -> None: + """Helper for TestManifestName: register a marketplace + plugin, create + a user in a group with a grant on that plugin.""" + t = datetime.now(timezone.utc) + _register_marketplace(conn, id=slug, registered_at=t, + plugins=[{"name": plugin, "version": "1.0"}]) + gid = _make_group(conn, name=f"G-{slug}") + _grant(conn, group_id=gid, marketplace=slug, plugin=plugin) + _make_user(conn, user_id=user_id, email=f"{user_id}@x") + _add_member(conn, user_id=user_id, group_id=gid) + + +class TestManifestName: + """resolve_allowed_plugins must surface the plugin's authoritative name + from its own .claude-plugin/plugin.json. Claude Code's /plugin UI looks + a loaded plugin back up against its catalog by plugin.json name; if the + synth marketplace.json's `name` doesn't match, the Components panel + errors with "Plugin not found in marketplace".""" + + def test_manifest_name_from_plugin_json(self, db_conn, tmp_path): + from src.marketplace_filter import resolve_allowed_plugins + _seed_grant_and_user(db_conn, slug="mkt", plugin="dirname") + plugin_dir = tmp_path / "marketplaces" / "mkt" / "plugins" / "dirname" + (plugin_dir / ".claude-plugin").mkdir(parents=True) + (plugin_dir / ".claude-plugin" / "plugin.json").write_text( + json.dumps({"name": "actual-name", "version": "1.0"}), + encoding="utf-8", + ) + + result = resolve_allowed_plugins(db_conn, {"id": "u-mn"}) + assert len(result) == 1 + assert result[0]["manifest_name"] == "actual-name" + # prefixed_name is unchanged — it drives the on-disk dir layout. + assert result[0]["prefixed_name"] == "mkt-dirname" + assert result[0]["original_name"] == "dirname" + + def test_manifest_name_falls_back_when_plugin_json_missing(self, db_conn, tmp_path): + from src.marketplace_filter import resolve_allowed_plugins + _seed_grant_and_user(db_conn, slug="mkt", plugin="myplugin") + # No plugin_dir on disk at all → falls back to upstream name. + result = resolve_allowed_plugins(db_conn, {"id": "u-mn"}) + assert len(result) == 1 + assert result[0]["manifest_name"] == "myplugin" + + def test_manifest_name_falls_back_on_malformed_plugin_json(self, db_conn, tmp_path): + from src.marketplace_filter import resolve_allowed_plugins + _seed_grant_and_user(db_conn, slug="mkt", plugin="myplugin") + plugin_dir = tmp_path / "marketplaces" / "mkt" / "plugins" / "myplugin" + (plugin_dir / ".claude-plugin").mkdir(parents=True) + (plugin_dir / ".claude-plugin" / "plugin.json").write_text( + "{ this is : not json", encoding="utf-8", + ) + + result = resolve_allowed_plugins(db_conn, {"id": "u-mn"}) + assert len(result) == 1 + assert result[0]["manifest_name"] == "myplugin" + + def test_manifest_name_falls_back_when_name_field_missing(self, db_conn, tmp_path): + from src.marketplace_filter import resolve_allowed_plugins + _seed_grant_and_user(db_conn, slug="mkt", plugin="myplugin") + plugin_dir = tmp_path / "marketplaces" / "mkt" / "plugins" / "myplugin" + (plugin_dir / ".claude-plugin").mkdir(parents=True) + (plugin_dir / ".claude-plugin" / "plugin.json").write_text( + json.dumps({"version": "1.0"}), encoding="utf-8", + ) + + result = resolve_allowed_plugins(db_conn, {"id": "u-mn"}) + assert len(result) == 1 + assert result[0]["manifest_name"] == "myplugin" + + # ETag tests (unchanged from v11) — still uses the in-process compute_etag helper. diff --git a/tests/test_marketplace_server_git.py b/tests/test_marketplace_server_git.py index cdc604c..20f8751 100644 --- a/tests/test_marketplace_server_git.py +++ b/tests/test_marketplace_server_git.py @@ -45,11 +45,18 @@ def git_env(e2e_env, monkeypatch): data_dir = e2e_env["data_dir"] - # Plugin folders on disk + # Plugin folders on disk — each ships a real .claude-plugin/plugin.json + # so the bare repo's synth marketplace.json picks up the plugin's + # authoritative name (matches what real upstream marketplaces do, and + # exercises the manifest_name resolution path). for slug, plug in [("mkt-a", "plug-x"), ("mkt-b", "plug-y")]: d = data_dir / "marketplaces" / slug / "plugins" / plug d.mkdir(parents=True, exist_ok=True) (d / "CLAUDE.md").write_text(f"# {plug}\n", encoding="utf-8") + (d / ".claude-plugin").mkdir() + (d / ".claude-plugin" / "plugin.json").write_text( + json.dumps({"name": plug, "version": "1.0"}), encoding="utf-8", + ) conn = get_system_db() try: @@ -248,3 +255,37 @@ class TestGitSmartHttp: # Two different cache entries (different RBAC views) entries = [p for p in cache.iterdir() if p.is_dir() and p.name.endswith(".git")] assert len(entries) == 2 + + def test_bare_repo_manifest_uses_plugin_json_name(self, git_env): + """The bare repo's .claude-plugin/marketplace.json must list each + plugin under the name declared in its own plugin.json (not the + slug-prefixed dir name). Otherwise Claude Code's /plugin UI can't + link the loaded plugin back to its catalog entry.""" + from dulwich.repo import Repo + + c = git_env["client"] + c.get( + "/marketplace.git/info/refs?service=git-upload-pack", + headers={"Authorization": _basic("x", git_env["admin_pat"])}, + ) + cache = git_env["data_dir"] / "marketplaces" / "git-cache" + bare = next(p for p in cache.iterdir() if p.is_dir() and p.name.endswith(".git")) + + repo = Repo(str(bare)) + try: + head = repo[repo.refs[b"HEAD"]] + tree = repo[head.tree] + # dulwich tree.items() yields TreeEntry tuples (path, mode, sha) + cp_entry = next(e for e in tree.items() if e.path == b".claude-plugin") + cp_subtree = repo[cp_entry.sha] + manifest_entry = next( + e for e in cp_subtree.items() if e.path == b"marketplace.json" + ) + manifest = json.loads(repo[manifest_entry.sha].data.decode("utf-8")) + finally: + repo.close() + + names = {p["name"] for p in manifest["plugins"]} + assert names == {"plug-x", "plug-y"} + sources = {p["source"] for p in manifest["plugins"]} + assert sources == {"./plugins/mkt-a-plug-x", "./plugins/mkt-b-plug-y"} diff --git a/tests/test_marketplace_server_zip.py b/tests/test_marketplace_server_zip.py index ed922ae..452d6a9 100644 --- a/tests/test_marketplace_server_zip.py +++ b/tests/test_marketplace_server_zip.py @@ -45,7 +45,10 @@ def marketplace_env(e2e_env, monkeypatch): data_dir = e2e_env["data_dir"] - # Plugin folders on disk + # Plugin folders on disk — each ships a real .claude-plugin/plugin.json + # so the synth marketplace.json picks up the plugin's authoritative name + # (matches what real upstream marketplaces do, and exercises the + # manifest_name resolution path). for slug, plug in [("mkt-a", "plug-x"), ("mkt-b", "plug-y"), ("mkt-b", "plug-z")]: d = data_dir / "marketplaces" / slug / "plugins" / plug d.mkdir(parents=True, exist_ok=True) @@ -55,6 +58,10 @@ def marketplace_env(e2e_env, monkeypatch): skills = d / "skills" skills.mkdir() (skills / "hello.md").write_text(f"skill for {plug}", encoding="utf-8") + (d / ".claude-plugin").mkdir() + (d / ".claude-plugin" / "plugin.json").write_text( + json.dumps({"name": plug, "version": "1.0"}), encoding="utf-8", + ) # DB setup conn = get_system_db() @@ -141,8 +148,15 @@ class TestMarketplaceInfo: resp = c.get("/marketplace/info", headers=_auth(marketplace_env["admin_token"])) assert resp.status_code == 200 info = resp.json() + # `name` in /marketplace/info mirrors what the synth manifest + # serves — the plugin's authoritative manifest_name (unprefixed + # in this fixture because plugin.json sets name=). names = {p["name"] for p in info["plugins"]} - assert names == {"mkt-a-plug-x", "mkt-b-plug-y", "mkt-b-plug-z"} + assert names == {"plug-x", "plug-y", "plug-z"} + # prefixed_name is exposed alongside so operators can still + # disambiguate a plugin's source marketplace. + prefixed = {p["prefixed_name"] for p in info["plugins"]} + assert prefixed == {"mkt-a-plug-x", "mkt-b-plug-y", "mkt-b-plug-z"} assert "Admin" in info["groups"] assert info["marketplace_name"] == "agnes" assert info["plugin_count"] == 3 @@ -153,7 +167,7 @@ class TestMarketplaceInfo: assert resp.status_code == 200 info = resp.json() names = {p["name"] for p in info["plugins"]} - assert names == {"mkt-b-plug-y"} + assert names == {"plug-y"} assert "TestGroup" in info["groups"] def test_user_with_no_groups_sees_empty_payload(self, marketplace_env): @@ -185,9 +199,12 @@ class TestMarketplaceZip: assert ".claude-plugin/marketplace.json" in zip_contents manifest = json.loads(zip_contents[".claude-plugin/marketplace.json"]) assert manifest["name"] == "agnes" + # `name` is the plugin's authoritative name from plugin.json — the + # fixture writes plugin.json with name=, so unprefixed. names = {p["name"] for p in manifest["plugins"]} - assert names == {"mkt-a-plug-x", "mkt-b-plug-y", "mkt-b-plug-z"} - # source paths flattened to prefixed names + assert names == {"plug-x", "plug-y", "plug-z"} + # source paths stay slug-prefixed so cross-marketplace dirs don't + # collide on disk in the flat ZIP / git tree layout. sources = {p["source"] for p in manifest["plugins"]} assert sources == { "./plugins/mkt-a-plug-x", @@ -198,6 +215,9 @@ class TestMarketplaceZip: assert "plugins/mkt-a-plug-x/CLAUDE.md" in zip_contents assert "plugins/mkt-b-plug-y/CLAUDE.md" in zip_contents assert "plugins/mkt-b-plug-z/skills/hello.md" in zip_contents + # plugin.json is included in each plugin tree so Claude Code can + # resolve the loaded plugin's namespace from it. + assert "plugins/mkt-a-plug-x/.claude-plugin/plugin.json" in zip_contents def test_analyst_zip_contains_only_granted(self, marketplace_env): c = marketplace_env["client"] @@ -300,3 +320,30 @@ class TestMarketplaceZip: ) assert resp.status_code == 200 assert resp.headers["content-type"] == "application/zip" + + def test_manifest_falls_back_when_plugin_json_missing(self, marketplace_env): + """If a plugin's .claude-plugin/plugin.json is absent, the synth + manifest falls back to the upstream marketplace.json's plugin name + (= mp.name in DB).""" + from app.marketplace_server.packager import invalidate_etag_cache + + c = marketplace_env["client"] + # Remove plug-x's plugin.json on disk + target = ( + marketplace_env["data_dir"] + / "marketplaces" + / "mkt-a" + / "plugins" + / "plug-x" + / ".claude-plugin" + / "plugin.json" + ) + target.unlink() + invalidate_etag_cache() + + resp = c.get("/marketplace.zip", headers=_auth(marketplace_env["admin_token"])) + assert resp.status_code == 200 + zip_contents = _read_zip(resp.content) + manifest = json.loads(zip_contents[".claude-plugin/marketplace.json"]) + plug_x = next(p for p in manifest["plugins"] if p["source"] == "./plugins/mkt-a-plug-x") + assert plug_x["name"] == "plug-x"