ultanio/cobot

Fork 4

Web plugin: extension point architecture (following CLI pattern) #80

New issue

Closed

opened 2026-02-22 13:16:30 +00:00 by nazim · 2 comments

nazim commented

2026-02-22 13:16:30 +00:00

Contributor

Problem

The web admin dashboard (#48) needs an architecture that lets plugins contribute panels, routes, and settings. The original PR (#48) used base class methods (web_panels(), web_settings(), web_routes()) — the same anti-pattern we just fixed for CLI in #75/#78.

Lesson from CLI Refactor (#78)

The CLI plugin refactor proved the pattern:

Create a plugin that defines extension points
Contributing plugins declare implements in their PluginMeta
Discovery via registry.get_implementations() — no base class methods, no raw iteration
The plugin graph shows all contributions as implements edges

Proposed: Web Plugin with Extension Points

Web plugin

class WebPlugin(Plugin):
    meta = PluginMeta(
        id="web",
        version="1.0.0",
        extension_points=["web.panels", "web.routes", "web.settings"],
        dependencies=["config"],
        priority=30,
    )

Extension point contracts

web.panels — contribute a panel to the admin dashboard:

def get_web_panels(self) -> list[dict]:
    """Return panel definitions for the web admin dashboard.
    
    Each dict: {
        "id": "plugin-graph",
        "title": "Plugin Graph",
        "icon": "🔌",
        "template": "plugins.html",   # or None if using route
        "route": "/admin/plugins",     # URL path
        "nav_group": "System",         # nav sidebar grouping
        "priority": 10,                # ordering within group
    }
    """

web.routes — contribute HTTP routes:

def get_web_routes(self) -> list[dict]:
    """Return route definitions.
    
    Each dict: {
        "method": "GET",
        "path": "/api/plugins/graph",
        "handler": self.handle_graph_api,
    }
    """

web.settings — contribute a settings section:

def get_web_settings(self) -> dict | None:
    """Return settings panel definition, or None."""

Example: Plugin graph as a web panel

The inspect functionality (now in PluginRegistry.inspect() via #77) becomes a panel:

# In the web plugin itself (built-in admin panel):
implements = {"web.panels": "get_web_panels"}

def get_web_panels(self):
    return [{
        "id": "plugin-graph",
        "title": "Plugin Graph",
        "icon": "🔌",
        "route": "/admin/plugins",
        "nav_group": "System",
    }]

Or any other plugin could contribute its own panel — wallet shows balance, knowledge shows search, logger shows recent logs, etc.

Web plugin startup flow

async def start(self):
    # Collect all panel contributors
    panels = []
    for pid, plugin, method in self._registry.get_implementations("web.panels"):
        panel_defs = getattr(plugin, method)()
        panels.extend(panel_defs)
    
    # Collect all route contributors  
    routes = []
    for pid, plugin, method in self._registry.get_implementations("web.routes"):
        route_defs = getattr(plugin, method)()
        routes.extend(route_defs)
    
    # Build nav, register routes, start Starlette
    self._app = build_app(panels, routes)
    await self._start_server()

Technology choices (from #48)

Starlette — async, lightweight, no Django overhead
HTMX — server-rendered with sprinkles of interactivity
Pico CSS — minimal, classless styling
D3.js — for the plugin graph (already built in #54/#77)

Acceptance Criteria

web plugin created with extension_points: ["web.panels", "web.routes", "web.settings"]
No web_panels() / web_settings() / web_routes() on Plugin base class
At least one panel contributed via implements (plugin graph)
registry.get_implementations() used for discovery
Plugin graph visible as admin panel with D3.js visualization
Starlette server starts on configured port
Plugin graph (PluginRegistry.inspect()) shows web contributions as implements edges

#48 — Original web admin PR (provides the UI prototype)
#75/#78 — CLI extension point refactor (the pattern to follow)
#54/#77 — Plugin introspection + PluginRegistry.inspect()
#59 — Plugin Design Guide (Principle 3: adding a plugin never requires editing another)

## Problem The web admin dashboard (#48) needs an architecture that lets plugins contribute panels, routes, and settings. The original PR (#48) used base class methods (`web_panels()`, `web_settings()`, `web_routes()`) — the same anti-pattern we just fixed for CLI in #75/#78. ## Lesson from CLI Refactor (#78) The CLI plugin refactor proved the pattern: 1. Create a plugin that defines extension points 2. Contributing plugins declare `implements` in their PluginMeta 3. Discovery via `registry.get_implementations()` — no base class methods, no raw iteration 4. The plugin graph shows all contributions as `implements` edges ## Proposed: Web Plugin with Extension Points ### Web plugin ```python class WebPlugin(Plugin): meta = PluginMeta( id="web", version="1.0.0", extension_points=["web.panels", "web.routes", "web.settings"], dependencies=["config"], priority=30, ) ``` ### Extension point contracts **`web.panels`** — contribute a panel to the admin dashboard: ```python def get_web_panels(self) -> list[dict]: """Return panel definitions for the web admin dashboard. Each dict: { "id": "plugin-graph", "title": "Plugin Graph", "icon": "🔌", "template": "plugins.html", # or None if using route "route": "/admin/plugins", # URL path "nav_group": "System", # nav sidebar grouping "priority": 10, # ordering within group } """ ``` **`web.routes`** — contribute HTTP routes: ```python def get_web_routes(self) -> list[dict]: """Return route definitions. Each dict: { "method": "GET", "path": "/api/plugins/graph", "handler": self.handle_graph_api, } """ ``` **`web.settings`** — contribute a settings section: ```python def get_web_settings(self) -> dict | None: """Return settings panel definition, or None.""" ``` ### Example: Plugin graph as a web panel The inspect functionality (now in `PluginRegistry.inspect()` via #77) becomes a panel: ```python # In the web plugin itself (built-in admin panel): implements = {"web.panels": "get_web_panels"} def get_web_panels(self): return [{ "id": "plugin-graph", "title": "Plugin Graph", "icon": "🔌", "route": "/admin/plugins", "nav_group": "System", }] ``` Or any other plugin could contribute its own panel — wallet shows balance, knowledge shows search, logger shows recent logs, etc. ### Web plugin startup flow ```python async def start(self): # Collect all panel contributors panels = [] for pid, plugin, method in self._registry.get_implementations("web.panels"): panel_defs = getattr(plugin, method)() panels.extend(panel_defs) # Collect all route contributors routes = [] for pid, plugin, method in self._registry.get_implementations("web.routes"): route_defs = getattr(plugin, method)() routes.extend(route_defs) # Build nav, register routes, start Starlette self._app = build_app(panels, routes) await self._start_server() ``` ### Technology choices (from #48) - **Starlette** — async, lightweight, no Django overhead - **HTMX** — server-rendered with sprinkles of interactivity - **Pico CSS** — minimal, classless styling - **D3.js** — for the plugin graph (already built in #54/#77) ## Acceptance Criteria - [ ] `web` plugin created with `extension_points: ["web.panels", "web.routes", "web.settings"]` - [ ] No `web_panels()` / `web_settings()` / `web_routes()` on Plugin base class - [ ] At least one panel contributed via `implements` (plugin graph) - [ ] `registry.get_implementations()` used for discovery - [ ] Plugin graph visible as admin panel with D3.js visualization - [ ] Starlette server starts on configured port - [ ] Plugin graph (`PluginRegistry.inspect()`) shows web contributions as `implements` edges ## Related - #48 — Original web admin PR (provides the UI prototype) - #75/#78 — CLI extension point refactor (the pattern to follow) - #54/#77 — Plugin introspection + `PluginRegistry.inspect()` - #59 — Plugin Design Guide (Principle 3: adding a plugin never requires editing another)

doxios commented

2026-02-22 13:19:51 +00:00

Collaborator

Architecture Review: Issue #80 — Web Plugin Extension Points

Verdict: APPROVE ✅

This proposal correctly applies the CLI extension point pattern from #78 to the web admin dashboard. The architecture is sound.

1. Extension Point Design ✅

Point	Assessment
`web.panels`	Well-defined. Contract includes id, title, icon, route, nav_group, priority — covers UI needs.
`web.routes`	Good. method/path/handler pattern is standard.
`web.settings`	Minimal but sufficient for v1. May need expansion later.

Suggestion: Consider defining TypedDict classes for the return contracts (like PanelDef, RouteDef) in interfaces.py. This provides IDE completion and enables static type checking:

class PanelDef(TypedDict):
    id: str
    title: str
    icon: str
    route: str
    nav_group: str
    priority: NotRequired[int]
    template: NotRequired[str]

2. Architecture Fit ✅

CLI pattern correctly followed: extension_points declaration → plugins use implements → discovery via registry.get_implementations() — exactly right.
No conflicts: No existing plugins claim similar extension points.
Priority 30: Perfect. This is the Aggregators band (30-49). Web aggregates panels/routes from other plugins, same as tools plugin aggregates tool definitions.
Dependencies ["config"]: Correct — needs config for port/host settings.

3. Implementation Concerns ⚠️

Missing from proposal:

Shutdown handling: How does the Starlette server stop gracefully? The stop() method needs to shutdown uvicorn:

async def stop(self) -> None:
    if self._server:
        self._server.should_exit = True
        await self._server.shutdown()

Authentication: Admin routes (/admin/*) need auth. Suggest a config option:

web:
  host: 127.0.0.1  # localhost-only by default (safe)
  port: 8080
  auth:
    enabled: true
    token: ${WEB_ADMIN_TOKEN}  # env var

And middleware that checks Authorization: Bearer <token> for /admin/* paths.

Error handling: What happens if a plugin's get_web_routes() raises? Should log and continue, not crash the server.

4. Acceptance Criteria ✅

All ACs are testable:

Plugin creation — unit test
No base class methods — grep test
Panel via implements — integration test
get_implementations() usage — code review
D3 graph visible — e2e test
Server starts — integration test
inspect() shows edges — unit test

Missing AC (suggest adding):

stop() cleanly shuts down the Starlette server
localhost-only binding by default (security)

5. Scope Assessment

Verdict: Single Story ✅

This is appropriately scoped. It defines:

The web plugin skeleton with extension points
One panel (plugin graph) as proof of concept
Starlette server lifecycle

It does NOT include:

All panels from #48 (those become follow-up stories)
All settings pages (follow-up)
All CLI web commands (separate story)

Recommended follow-up stories:

web: add wallet panel (implements web.panels)
web: add knowledge panel
web: add logger panel
web: add settings panels

Summary

Criteria	Status
Extension points	✅ Well-designed
Architecture fit	✅ Follows CLI pattern
Priority band	✅ 30 (Aggregators)
Implementation	⚠️ Add shutdown + auth
Acceptance criteria	✅ Testable (add 2 more)
Scope	✅ Single story

Approved — proceed with implementation. Address shutdown and auth in the PR.

Reviewed by sub-agent review-80 per Plugin Design Guide principles

## Architecture Review: Issue #80 — Web Plugin Extension Points **Verdict: APPROVE** ✅ This proposal correctly applies the CLI extension point pattern from #78 to the web admin dashboard. The architecture is sound. --- ### 1. Extension Point Design ✅ | Point | Assessment | |-------|------------| | `web.panels` | Well-defined. Contract includes id, title, icon, route, nav_group, priority — covers UI needs. | | `web.routes` | Good. method/path/handler pattern is standard. | | `web.settings` | Minimal but sufficient for v1. May need expansion later. | **Suggestion:** Consider defining `TypedDict` classes for the return contracts (like `PanelDef`, `RouteDef`) in `interfaces.py`. This provides IDE completion and enables static type checking: ```python class PanelDef(TypedDict): id: str title: str icon: str route: str nav_group: str priority: NotRequired[int] template: NotRequired[str] ``` ### 2. Architecture Fit ✅ - **CLI pattern correctly followed:** `extension_points` declaration → plugins use `implements` → discovery via `registry.get_implementations()` — exactly right. - **No conflicts:** No existing plugins claim similar extension points. - **Priority 30:** Perfect. This is the Aggregators band (30-49). Web aggregates panels/routes from other plugins, same as `tools` plugin aggregates tool definitions. - **Dependencies `["config"]`:** Correct — needs config for port/host settings. ### 3. Implementation Concerns ⚠️ **Missing from proposal:** 1. **Shutdown handling:** How does the Starlette server stop gracefully? The `stop()` method needs to shutdown uvicorn: ```python async def stop(self) -> None: if self._server: self._server.should_exit = True await self._server.shutdown() ``` 2. **Authentication:** Admin routes (`/admin/*`) need auth. Suggest a config option: ```yaml web: host: 127.0.0.1 # localhost-only by default (safe) port: 8080 auth: enabled: true token: ${WEB_ADMIN_TOKEN} # env var ``` And middleware that checks `Authorization: Bearer <token>` for `/admin/*` paths. 3. **Error handling:** What happens if a plugin's `get_web_routes()` raises? Should log and continue, not crash the server. ### 4. Acceptance Criteria ✅ All ACs are testable: - [x] Plugin creation — unit test - [x] No base class methods — grep test - [x] Panel via implements — integration test - [x] get_implementations() usage — code review - [x] D3 graph visible — e2e test - [x] Server starts — integration test - [x] inspect() shows edges — unit test **Missing AC (suggest adding):** - [ ] `stop()` cleanly shuts down the Starlette server - [ ] localhost-only binding by default (security) ### 5. Scope Assessment **Verdict: Single Story** ✅ This is appropriately scoped. It defines: 1. The `web` plugin skeleton with extension points 2. One panel (plugin graph) as proof of concept 3. Starlette server lifecycle It does NOT include: - All panels from #48 (those become follow-up stories) - All settings pages (follow-up) - All CLI web commands (separate story) **Recommended follow-up stories:** - `web: add wallet panel` (implements `web.panels`) - `web: add knowledge panel` - `web: add logger panel` - `web: add settings panels` --- ### Summary | Criteria | Status | |----------|--------| | Extension points | ✅ Well-designed | | Architecture fit | ✅ Follows CLI pattern | | Priority band | ✅ 30 (Aggregators) | | Implementation | ⚠️ Add shutdown + auth | | Acceptance criteria | ✅ Testable (add 2 more) | | Scope | ✅ Single story | **Approved** — proceed with implementation. Address shutdown and auth in the PR. --- *Reviewed by sub-agent `review-80` per Plugin Design Guide principles*

nazim commented

2026-02-22 13:22:02 +00:00

Author

Contributor

Good review. Agreed on all three implementation concerns:

Shutdown — will add graceful uvicorn shutdown in stop()
Auth — localhost-only by default + optional Bearer token for remote access
Error handling — get_web_panels()/get_web_routes() wrapped in try/except, log and skip

Also adding the 2 suggested ACs:

stop() cleanly shuts down Starlette
localhost-only binding by default

TypedDict for contracts (PanelDef, RouteDef) — good idea, will add to interfaces.py.

Ready to implement.

Good review. Agreed on all three implementation concerns: 1. **Shutdown** — will add graceful uvicorn shutdown in `stop()` 2. **Auth** — localhost-only by default + optional Bearer token for remote access 3. **Error handling** — `get_web_panels()`/`get_web_routes()` wrapped in try/except, log and skip Also adding the 2 suggested ACs: - `stop()` cleanly shuts down Starlette - localhost-only binding by default `TypedDict` for contracts (`PanelDef`, `RouteDef`) — good idea, will add to `interfaces.py`. Ready to implement.