diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e2ec8a24..7b64a74c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,11 +7,11 @@ # frontend - vitest + Vue build # lean-build - ADR-013: a per-site build carries only its plugins # api-contract - the published spec matches the code that serves it -# migrations-mysql - the REAL multi-site deploy path: fresh flask db upgrade -# + every plugin's chain on utf8mb4 MySQL 8, idempotent on -# a second run. The pytest suite only exercises SQLite -# create_all(), so this is what catches an Alembic -# regression on MySQL before it ships. +# +# The MySQL-8 migration job is NOT here; it runs on the internal CI server. See +# the note at the bottom of this file. It was listed here for a while after it +# moved, which reads as coverage this workflow does not have - do not re-add it +# to this list without re-adding the job to a runner that can host it. name: CI @@ -74,6 +74,27 @@ jobs: tests/test_openapi_spec.py \ tests/test_openapi_examples.py \ tests/test_core/test_public_read_surface.py + - name: Seed the plugin registry the generator needs + # gen_openapi.py refuses to run while a bundled plugin is disabled, and + # instance/plugins.json is gitignored, so a clean checkout has none. + # The step used to pass only because the pytest step above writes that + # file at import time and never removes it - green by accident of step + # order, and red the moment the steps are reordered, with an error + # about disabled plugins that has nothing to do with the spec. + run: | + python - <<'PY' + import datetime, json, pathlib + names = sorted(d.name for d in pathlib.Path('plugins').iterdir() + if (d / 'manifest.json').is_file()) + now = datetime.datetime.now().isoformat() + out = pathlib.Path('instance'); out.mkdir(exist_ok=True) + (out / 'plugins.json').write_text(json.dumps({'plugins': { + n: {'name': n, 'version': '1.0.0', 'installed_at': now, + 'enabled': True, 'migrations_applied': [], 'config': {}} + for n in names}}, indent=2)) + print('registry seeded with', len(names), 'plugins') + PY + - name: The spec must be regenerated, not just passing # --refresh is derived from the code, so if it changes anything the # committed spec was stale. Fail here rather than let a generated client @@ -87,73 +108,6 @@ jobs: exit 1 fi - migrations-mysql: - # THE JOB THE HEADER PROMISED AND NOBODY WROTE. Until now this existed only - # as a comment claiming to cover "the REAL multi-site deploy path", which is - # worse than no comment: it is exactly the false confidence that let a - # migration ship which could not build a schema on a fresh MySQL database. - # - # The pytest suite runs on SQLite, and SQLite accepts a foreign key onto a - # table that does not exist. MySQL does not. That difference is invisible to - # every other job here. - runs-on: arc-runner-set - services: - mysql: - image: mysql:8.4 - env: - MYSQL_ROOT_PASSWORD: ci - MYSQL_DATABASE: shopdb_flask - options: >- - --health-cmd="mysqladmin ping -h 127.0.0.1 -u root -pci" - --health-interval=10s --health-timeout=5s --health-retries=10 - ports: - - 3306:3306 - env: - DATABASE_URL: mysql+pymysql://root:ci@127.0.0.1:3306/shopdb_flask?charset=utf8mb4 - FLASK_APP: shopdb - SECRET_KEY: ci-not-a-secret - JWT_SECRET_KEY: ci-not-a-secret - CORS_ORIGINS: http://localhost - steps: - - uses: actions/checkout@v5 - - uses: actions/setup-python@v6 - with: - python-version: '3.14' - cache: pip - - run: pip install -r requirements.txt - - name: Fresh install, exactly as the README tells a site to do it - run: | - flask db upgrade - flask plugin upgrade-all - flask seed permissions - flask seed settings - flask seed reference-data - - name: Enable every bundled plugin and migrate it - # A plugin's chain only runs once the plugin is ENABLED, so a core - # migration that depends on a plugin-owned table passes an empty install - # and fails a real one. Enabling them here is what reproduces that. - run: | - python - <<'PY' - import json, os, pathlib, datetime - root = pathlib.Path('plugins') - names = sorted(d.name for d in root.iterdir() - if (d / 'manifest.json').is_file()) - now = datetime.datetime.now().isoformat() - state = {'plugins': {n: {'name': n, 'version': '1.0.0', - 'installed_at': now, 'enabled': True, - 'migrations_applied': [], 'config': {}} - for n in names}} - out = pathlib.Path('instance'); out.mkdir(exist_ok=True) - (out / 'plugins.json').write_text(json.dumps(state, indent=2)) - print('enabled', len(names), 'plugins') - PY - flask plugin upgrade-all - - name: Re-run to prove it is idempotent - run: | - flask db upgrade - flask plugin upgrade-all - flask seed permissions - gates: # Prove the gates RAN. Several skip themselves when the file they read is # absent, which is correct in this checkout (docs/ becomes the wiki) and was diff --git a/scripts/gen_openapi.py b/scripts/gen_openapi.py index 9a82a0c1..4ee165f4 100644 --- a/scripts/gen_openapi.py +++ b/scripts/gen_openapi.py @@ -1199,7 +1199,7 @@ def _inventory_app(): raise SystemExit( 'these bundled plugins are disabled, so their routes would be ' 'missing\nfrom the inventory: %s\nenable them and re-run:\n' - ' FLASK_APP=shopdb venv/bin/flask plugins enable %s' + ' FLASK_APP=shopdb venv/bin/flask plugin enable %s' % (', '.join(off), off[0])) return app diff --git a/shopdb/core/api/docs.py b/shopdb/core/api/docs.py index c975765a..4ca260ac 100644 --- a/shopdb/core/api/docs.py +++ b/shopdb/core/api/docs.py @@ -364,9 +364,22 @@ def operations_index(): if not isinstance(op, dict) or method.upper() not in _HTTP_METHODS: continue tag = (op.get('tags') or ['-'])[0] - auth = 'public' if op.get('security') == [] else ( - ','.join(sorted({k for s in (op.get('security') or []) - for k in s})) or 'bearer') + # security == [] means public. A list containing an EMPTY dict + # means the scheme is optional - the route works signed out and + # returns more when authenticated - and 14 routes here are that + # shape. Collapsing them to the scheme name told a caller reading + # only the index that it needed a token for a route that does not + # require one. + security = op.get('security') + if security == []: + auth = 'public' + else: + entries = security or [] + schemes = sorted({k for entry in entries for k in entry}) + optional = any(not entry for entry in entries) + auth = ','.join(schemes) or 'bearer' + if optional: + auth += ',optional' summary = ' '.join((op.get('summary') or '').split()) rows.append(f'{method.upper():<6} {path:<52} [{tag}] ({auth}) {summary}') header = ( diff --git a/tools/mcp/shopdb_mcp/client.py b/tools/mcp/shopdb_mcp/client.py index f3264cba..b8b6c8ce 100644 --- a/tools/mcp/shopdb_mcp/client.py +++ b/tools/mcp/shopdb_mcp/client.py @@ -16,6 +16,7 @@ import threading import time from dataclasses import dataclass from typing import Any +import http.client as http_client from urllib import error, parse, request # How long a fetched index or spec stays usable before it is re-read. Long @@ -86,7 +87,11 @@ class ShopdbClient: def request(self, method: str, path: str, params: dict | None = None, body: Any = None) -> tuple[int, Any]: """One HTTP call. Returns (status, parsed-body-or-text).""" - url = self.base_url + '/' + path.lstrip('/') + # Encode what a caller typed. A model filling an id into a templated + # path can easily produce a space or an accent; leaving those raw turns + # a legitimate request into an exception rather than a 404. '/' and the + # query delimiters stay literal so an already-formed path still works. + url = self.base_url + '/' + parse.quote(path.lstrip('/'), safe='/?=&%+,') if params: # Drop empty values rather than sending key= , which several list # endpoints treat as a filter on the empty string. @@ -102,8 +107,9 @@ class ShopdbClient: data = json.dumps(body).encode('utf-8') headers['Content-Type'] = 'application/json' - req = request.Request(url, data=data, headers=headers, method=method.upper()) try: + req = request.Request(url, data=data, headers=headers, + method=method.upper()) with self._opener().open(req, timeout=self.timeout) as response: return response.status, _decode(response.read(), response.headers.get('Content-Type')) @@ -113,6 +119,15 @@ class ShopdbClient: return exc.code, _decode(exc.read(), exc.headers.get('Content-Type')) except error.URLError as exc: raise ShopdbError(f'could not reach {self.base_url}: {exc.reason}') from exc + except (OSError, ValueError, http_client.HTTPException) as exc: + # Catching only the two urllib errors let ordinary mistakes escape + # as raw exceptions: a space in a path raises InvalidURL, a non + # ASCII path raises UnicodeEncodeError, a read that outruns the + # timeout raises TimeoutError. The caller then saw a bare "error + # executing tool" with no cause and nothing to try differently. + raise ShopdbError( + f'{method.upper()} {path} could not be sent: ' + f'{type(exc).__name__}: {exc}') from exc def _cached(self, key: str, fetch): with self._lock: diff --git a/tools/mcp/shopdb_mcp/server.py b/tools/mcp/shopdb_mcp/server.py index 56041dc8..c046fec6 100644 --- a/tools/mcp/shopdb_mcp/server.py +++ b/tools/mcp/shopdb_mcp/server.py @@ -54,11 +54,16 @@ def _fail(message: str) -> str: @mcp.tool() def shopdb_whoami() -> str: - """Check which ShopDB instance this server targets and what the token can do. + """Check which ShopDB instance this server targets and who the token is. - Call this first. It confirms the instance is reachable, the credential is - valid, and - importantly - what the token is scoped to, which determines - which other calls will succeed. + Call this first: it confirms the instance is reachable and the credential is + valid. + + IMPORTANT: the permissions reported are the ACCOUNT's, not the token's. A + scoped personal access token carries its own narrower scope list, which the + server enforces but does not publish, so a call can still be refused with + 403 even though the account shown here holds the permission. Treat this as + "who am I", not "what may I do". """ try: c = client() @@ -81,7 +86,14 @@ def shopdb_whoami() -> str: 'instance': c.base_url, 'username': user.get('username'), 'roles': user.get('roles'), - 'permissions': len(user.get('permissions') or []), + # Named account_permissions, not permissions, because that is what it + # is. Calling it "permissions" next to a token invited exactly the + # wrong reading: that an admin account means every call will succeed, + # when a read-scoped token on that same account is refused a write. + 'account_permissions': len(user.get('permissions') or []), + 'note': ('permissions shown are the account\'s; a scoped token may ' + 'allow fewer, and a 403 from shopdb_call means the TOKEN ' + 'lacks the scope even if the account holds the permission'), 'operations_published': count, 'tls_verified': c.verify_tls, }, indent=2)