feat(plugins): backend-declared plugin names and frontend metadata

[hamza-56]

Part of: Plugin identity and display metadata are split across disconnected backend and frontend registries

First PR of the sequence proposed in #372 (hooks + API exposure). Frontend consumption and the drift gate follow separately, so this does not close #565 yet.

What

The backend becomes the single source of truth for a plugin's identity and display metadata: plugins declare their own name explicitly (nothing is derived from the class name anymore) and contribute display info, sidebar entry, and a frontend-app marker through per-concern hooks, which the user-plugins API exposes read-only.

Changes

• feat(plugins): plugins declare their own kebab-case name positionally via super().__init__(); the loader constructs classes with no arguments and stops deriving names from class names
• feat(plugins): add DISPLAY_INFO, SIDEBAR_ENTRIES, and FRONTEND_APPS hooks (sparkth/lib/frontend) with name-based lookup helpers
• feat(plugins): built-in plugins register their display info (canvas and open-edx finally get one), sidebar entries (chat, slack), and frontend-app markers (chat, google-drive, slack)
• feat(api): expose display, sidebar, and has_frontend read-only on UserPluginResponse; regenerate frontend/lib/api/generated.ts
• test(plugins): loader explicit-name contract, hook lookup helpers, per-plugin identity tests, API response coverage
• docs(plugins): replace the "Plugin Name Derivation" section with explicit declaration, document the frontend metadata hooks

How to Test

1. uv run pytest (new suites: tests/lib/test_frontend.py, tests/core/plugins/test_loader.py, sparkth/plugins/*/tests/test_plugin.py)
2. make backend.up.dev, authenticate, then GET /api/v1/user-plugins/: every plugin entry carries display, sidebar, and has_frontend; canvas and open-edx now have a display name and description
3. make docs: the Plugin authoring reference renders the new sparkth.lib.frontend section

Notes

• Breaking for plugin authors: a plugin __init__ no longer receives the name from the loader. Declare it explicitly: def __init__(self) -> None: super().__init__("my-app"). Old-style plugins are skipped at load time with an error in the logs.
• No migration needed: all five built-in plugins declare the exact names previously derived (canvas, chat, google-drive, open-edx, slack), so existing DB rows keep matching.
• Plugin names are now validated as kebab-case slugs; super().__init__() raises ValueError otherwise.
• No new env vars, no dependency changes.

This PR description was written with the assistance of an LLM (Claude).

[hamza-56]

@claude-review

[claude]

Claude finished @hamza-56's task in 2m 40s —— View job

---

Code Review — feat(plugins): backend-declared plugin names and frontend metadata

I read the full diff against origin/main (core loader/base/service, the new sparkth/lib/frontend hooks, the API changes + regenerated TS, all five plugin declarations, tests, and docs).

Overall: strong, coherent PR. The design mirrors the existing hook patterns (CONFIG_SCHEMAS, MCP_TOOLS) faithfully, tests are genuinely thorough (loader contract, hook lookups, per-plugin identity, and API-response coverage), docs are updated in step, and the generated.ts is regenerated. No blocking issues found. TDD/docs/exception-handling conventions from CLAUDE.md are respected. A few low-severity notes below.

---

🟢 Things done well

• The explicit-name contract is clean: plugin_class() + read instance.name, with distinct, well-logged failure paths for construction error, missing name, and duplicate name — each covered by a loader test.
• PLUGIN_NAME_PATTERN validation lives in one place (base.py) and is parametrized-tested for both accept and reject cases.
• Per-concern hooks (DISPLAY_INFO / SIDEBAR_ENTRIES / FRONTEND_APPS) instead of a monolithic manifest — good separation, and FrontendApp is honestly documented as a forward-looking marker.
• UserPluginResponse.for_plugin(...) centralizes metadata population so every route (list/get/create/update) stays consistent — nicely done, and the get/list tests assert the JSON shape end-to-end.
• Migration-safety reasoning is sound: all five built-ins declare the exact previously-derived slugs, so existing DB rows keep matching.

🟡 Low / suggestions (non-blocking)

1. Loader duplicate-skip doesn't roll back side effects — a duplicate plugin's __init__ runs fully (registering routers/config/tools/display) before the skip. It self-heals via the WeakKeyDictionary-backed hooks, but relies on GC timing vs. assemble_app. See the inline note on loader.py — a clarifying comment or explicit del would make the intent robust.
2. Name lookups are linear and called per-plugin — get_plugin_display_info / get_plugin_sidebar_entry / plugin_has_frontend each scan all hook items, and list_user_plugins calls all three per plugin (each iter_items also re-sorts). O(n·m) with tiny n, so purely cosmetic today; if the plugin count ever grows, a name-indexed lookup would be cleaner. Fine to leave as-is.
3. Doc/description nit — the google-drive DisplayInfo description ("All imported files from your connected plugins") reads like page copy rather than a description of the plugin's identity, unlike the others ("Canvas LMS integration…", "Open edX integration…"). Worth a second look for consistency with how DISPLAY_INFO is documented ("what the plugin does, in one line").

✅ Test coverage

New functionality is well covered: tests/core/plugins/test_loader.py (declared name, no derivation, duplicate, exploding/nameless/legacy skip), tests/lib/test_frontend.py (hook dataclasses + lookups + unknown-plugin None), per-plugin test_plugin.py for all five, and the API list/get tests assert display/sidebar/has_frontend including the populated case. The weak-key isolation trick (# noqa: F841 — keeps the entries alive) is a nice touch. No gaps that would block.

---

_{Note: I reviewed the code but did not run the suite in this session. · branch hamza/feat/plugin-frontend-metadata}

[claude]

Low / informational: by the time the duplicate-name check runs, the plugin's __init__ has already executed and registered its side effects into the global hooks (DISPLAY_INFO, CONFIG_SCHEMAS, MCP_TOOLS, and notably PLUGIN_ROUTERS via register_router). Skipping here only prevents storing the instance in _loaded_plugins; it does not roll back those registrations.

In practice this self-heals because every one of those hooks is a PluginHook backed by a WeakKeyDictionary — once the loop drops its only strong reference to the skipped instance, its entries disappear. But that leaves the cleanup dependent on GC timing relative to when assemble_app reads PLUGIN_ROUTERS. Under CPython refcounting it's collected immediately, so it's fine today; worth a one-line comment noting the reliance, or (belt-and-suspenders) del plugin_instance on the skip path to make the intent explicit.