Say what a plugin settings tab can actually do, and catch a declaration that cannot work
Some checks failed
CI / backend (push) Has been cancelled
CI / naming (push) Has been cancelled
CI / gates (push) Has been cancelled
CI / frontend (push) Has been cancelled
CI / lean-build (push) Has been cancelled

Finding F12 of the architectural review, checked against the code and found
worse than reported: the dynamic component import cannot be rewritten by the
bundler, so the built file ships the unresolved path and a plugin tab answers
404 in every production build, staged or not. It works only under the dev
server. Nothing shipping is affected, because the one plugin that declares a
tab also registers the same page as an ordinary route, and that is the one
sites reach.

- The ADR draft now carries a "Constraints as implemented" section: only the
  model catalog hub registers plugin tabs, registration happens when that hub
  mounts so a tab is not deep-linkable and does not survive a refresh, and the
  build finding above with the bundler's own warning. The two ways out are
  stated as the open decision rather than taken.
- The hook's docstring reads as instructions to a plugin author, and its
  example no longer names a path that does not exist at build time.
- A route declared for a hub that cannot render it is dropped with an error
  naming the plugin, the hub and the hubs that do work, and the same is logged
  at startup. Enabling the plugin still succeeds: a tab nobody can reach is no
  reason to take a working backend offline.
- Tests hold the two halves together: no bundled plugin may declare a hub the
  frontend does not implement, and the list in the server must equal the hubs
  that really register routes, so the two cannot drift apart again.
This commit is contained in:
cproudlock
2026-09-16 09:57:51 -04:00
parent 066a19052b
commit 1a6ac67cab
8 changed files with 380 additions and 14 deletions

View File

@@ -0,0 +1,160 @@
"""A plugin settings tab is only real if a hub is written to draw it.
`get_settings_routes()` (ADR-019) lets a plugin declare a settings tab and name
the hub it belongs to. The server accepts any hub string, but only a hub whose
.vue page asks /api/plugins/settings-routes and calls router.addRoute() ever
draws one. Today that is Model Catalog and nothing else.
The failure that gets is the quiet kind: the plugin is right, the API is right,
the response carries the tab, and the person never sees it. So the supported set
is named once in shopdb/plugins/base.py, the API drops anything outside it with
an error in the log, and these tests hold the two ends together - no bundled
plugin may declare a hub that cannot draw it, and the list in the server may not
drift away from the hubs that actually register routes in the frontend.
"""
import re
from pathlib import Path
import pytest
from shopdb.plugins import plugin_manager
from shopdb.plugins.base import SETTINGS_HUBS_WITH_PLUGIN_TABS
REPO = Path(__file__).resolve().parents[2]
HUB_SOURCES = sorted((REPO / 'frontend' / 'src' / 'views' / 'settings')
.glob('*.vue'))
# router.addRoute('model-catalog', { ... }) - the parent whose children the hub
# adds to, which is also the id a route's `hub` field has to carry.
ADDROUTE = re.compile(r"router\.addRoute\(\s*['\"]([^'\"]+)['\"]")
# ...filter(r => r.hub === 'model-catalog')
HUB_FILTER = re.compile(r"\.hub\s*===\s*['\"]([^'\"]+)['\"]")
BUNDLED = sorted(path.parent.name
for path in (REPO / 'plugins').glob('*/manifest.json'))
@pytest.fixture
def declared_routes(app):
"""Every bundled plugin's declared settings routes, enabled or not.
Read from the classes rather than the loaded plugins: a plugin disabled on
this machine still ships, and a dead tab in it is still a defect.
"""
routes = {}
with app.app_context():
for name in BUNDLED:
cls = plugin_manager.loader.load_plugin_class(name)
assert cls is not None, f'Plugin class for {name} could not be loaded'
routes[name] = cls().get_settings_routes() or []
return routes
def test_the_sweep_actually_reads_something():
"""A gate that matches nothing passes forever."""
assert len(HUB_SOURCES) > 5, [path.name for path in HUB_SOURCES]
assert BUNDLED, 'no bundled plugins found - the glob is probably wrong'
assert any(ADDROUTE.search(path.read_text()) for path in HUB_SOURCES), \
'no hub registers plugin routes at all - the pattern is probably wrong'
def test_every_bundled_plugin_declares_a_hub_that_can_draw_it(declared_routes):
offenders = [
"%s: route '%s' names hub '%s'" % (name, route.get('name'),
route.get('hub'))
for name, routes in declared_routes.items()
for route in routes
if route.get('hub') not in SETTINGS_HUBS_WITH_PLUGIN_TABS
]
assert not offenders, (
'These settings tabs would be returned by the API and drawn by no one. '
'Move them to a hub that supports plugin tabs (%s), or write the '
'fetch-and-addRoute code into the hub page and add it to '
'SETTINGS_HUBS_WITH_PLUGIN_TABS:\n %s'
% (', '.join(SETTINGS_HUBS_WITH_PLUGIN_TABS), '\n '.join(offenders)))
def test_the_supported_hubs_are_the_ones_the_frontend_registers():
"""The server's list and the frontend's code cannot be allowed to drift.
A hub page supports plugin tabs when it does both halves: keeps the routes
whose `hub` is its own id, and registers them with addRoute. A page doing
only one of the two is worse than neither, so it counts as neither.
"""
registering = set()
for path in HUB_SOURCES:
source = path.read_text()
if not ADDROUTE.search(source):
continue
registering.update(HUB_FILTER.findall(source))
assert registering == set(SETTINGS_HUBS_WITH_PLUGIN_TABS), (
'SETTINGS_HUBS_WITH_PLUGIN_TABS says %s; the settings hubs that fetch '
'and register plugin routes are %s. Whichever one moved, the other has '
'to move with it.'
% (sorted(SETTINGS_HUBS_WITH_PLUGIN_TABS), sorted(registering)))
def test_a_declared_tab_is_also_reachable_by_a_static_route(declared_routes):
"""A settings tab cannot be the only way to a plugin's page.
The hub loads the tab with `import(`../../${tab.component}`)`, a template
with no static file extension. Vite's dynamic-import-vars plugin refuses to
rewrite that ("A file extension must be included in the static part of the
import"), so a production build emits the expression verbatim and the
browser asks for a .vue path that no build ships. The tab therefore works
under `npm run dev`, where the dev server serves sources by path, and not in
a built bundle.
Until that is decided one way or the other (ADR-019), a plugin that declares
a tab must also register the same page in its own routes.js, the way
computers does with /settings/operatingsystems - a static import, a real
URL, and the thing production actually uses.
"""
routesjs = re.compile(r"import\(\s*['\"]([^'\"]+\.vue)['\"]")
offenders = []
for name, routes in declared_routes.items():
source = REPO / 'plugins' / name / 'frontend' / 'routes.js'
statics = {Path(match).name
for match in routesjs.findall(source.read_text())} \
if source.exists() else set()
for route in routes:
component = Path(route.get('component') or '').name
if component not in statics:
offenders.append("%s: '%s' (%s) has no static route"
% (name, route.get('name'), component))
assert not offenders, (
'These settings pages are reachable only through a hub tab, whose '
'component import does not survive a production build. Give each one a '
'route in the plugin routes.js as well:\n ' + '\n '.join(offenders))
def test_a_route_for_an_unsupported_hub_is_dropped_and_logged(
app, client, db, monkeypatch, caplog, auth_headers):
manager = app.extensions.get('plugin_manager')
plugins = manager.get_all_plugins()
enabled = [name for name in plugins if manager.registry.is_enabled(name)]
assert enabled
def contribute():
return [
{'hub': 'model-catalog', 'path': 'drawn', 'name': 'drawn-route',
'tab': 'Drawn', 'component': 'drawn.vue'},
{'hub': 'asset-config', 'path': 'dead', 'name': 'dead-route',
'tab': 'Dead', 'component': 'dead.vue'},
]
monkeypatch.setattr(plugins[enabled[0]], 'get_settings_routes', contribute,
raising=False)
with caplog.at_level('ERROR'):
response = client.get('/api/plugins/settings-routes',
headers=auth_headers)
assert response.status_code == 200, response.get_json()
names = [route['name'] for route in response.get_json()['data']['routes']]
assert 'drawn-route' in names
assert 'dead-route' not in names
assert any('asset-config' in record.getMessage() for record in caplog.records), \
'the drop has to be loud, or it is the same silence in a new place'