ge-aerospace/shopdb-flask
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

openapi: emit a spec a machine can actually use

Browse Source 
The generated spec carried no `parameters` and no `requestBody` on any of its
372 operations. That is invalid OpenAPI 3.1, and the practical cost was worse
than the formal one: the MCP server builds its tools from this file, so every
tool had an empty input schema and silently dropped whatever the caller passed.
A request for one asset returned the list, and nothing anywhere reported an
error. All 118 templated paths now declare their path parameters, typed from the
Flask converter that named them, and write verbs declare a JSON body.

The body is an open object carrying the prose description rather than an
invented schema. The inventory describes bodies in sentences, and a field list
this generator guessed at would be worse than none - but "an object, described
here" is the difference between a client that can send a body and one that
cannot send anything.

Security was wrong on 123 operations. `jwt-optional` means "works logged out,
returns more logged in", which OpenAPI expresses as the empty requirement
alongside the scheme; publishing them as bearer-required told every reader that
a public endpoint needs a token.

Responses were one hardcoded 200, so a generated client had no idea a call could
fail. Every operation now documents the error envelope - and the envelope itself
is a defined schema, because its error nests under `data.error` rather than at
the top level, which is the single thing people get wrong when writing against
this API.

95 summaries were cut at 120 characters mid-word, which is what a tool picker
shows a user as the whole description of a call. They now end on a word.

Tests pin the shape rather than the prose. One of them contradicted an older
test that REQUIRED the contract version as a literal in PLUGIN-HOOKS.md - the
same copying that left nine documents stale - so that test now asserts the doc
points at the generated map instead.
This commit is contained in:
cproudlock
2026-08-14 15:56:27 -04:00
parent 05be4c4489
commit 36b0265668
4 changed files with 11728 additions and 618 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12017
docs/openapi.json
View File

File diff suppressed because it is too large Load Diff

182
scripts/gen_openapi.py
View File

@@ -58,14 +58,121 @@ def _norm_path(p):
def _security(auth):
"""OpenAPI security for one endpoint's documented auth string.
`jwt-optional` is the interesting case, and the common one - 121 endpoints.
It means "works logged out, returns more when logged in", which OpenAPI
expresses as a list containing BOTH the empty requirement and the scheme.
Publishing those as bearer-required told every reader, human and machine,
that a public endpoint needs a token.
"""
a = (auth or '').lower()
if a in ('none', 'public', ''):
if 'optional' in a:
return [{}, {'bearerAuth': []}]
if a in ('none', 'public', '') or a.startswith('none') or a.startswith('public'):
return []
if 'api-key' in a or 'api_key' in a or 'x-api-key' in a:
return [{'apiKeyAuth': []}]
return [{'bearerAuth': []}]
def _path_parameters(rawpath):
"""Path parameters, typed from the Flask converter that declared them.
The generator emitted no `parameters` at all, which makes the document
invalid OpenAPI 3.1 - and breaks its only machine consumer outright, because
a tool built from an operation with no parameters has nowhere to put the id
the caller supplied, so it is dropped in silence.
"""
parameters = []
for converter, name in re.findall(r'<(?:([^:>]+):)?([^>]+)>', rawpath or ''):
schema = {'type': 'integer'} if converter in ('int',) else {'type': 'string'}
if converter == 'float':
schema = {'type': 'number'}
if converter == 'path':
schema = {'type': 'string', 'description': 'may contain slashes'}
parameters.append({
'name': name,
'in': 'path',
'required': True,
'schema': schema,
})
return parameters
def _request_body(verb, entry):
"""A JSON body for the verbs that take one.
The inventory describes bodies in prose, not as schemas, so this does not
invent field names it cannot verify - it declares an object and carries the
prose. That is honest, and it is the difference between a machine client
that can send a body and one that cannot send anything at all.
"""
if verb not in ('post', 'put', 'patch'):
return None
described = entry.get('params') or ''
return {
'required': verb in ('post', 'put'),
'content': {
'application/json': {
'schema': {
'type': 'object',
'additionalProperties': True,
'description': described or 'See the endpoint description.',
}
}
},
}
def _responses(security, parameters):
"""The envelope, plus the failures a caller has to handle.
One hardcoded 200 was the whole response section, so a generated client had
no idea any call could fail, and 19 operations that do not return the
envelope at all were documented as if they did.
"""
responses = {
'200': {
'description': 'Success. Body is the success_response envelope: '
'{status, data, meta}.',
'content': {'application/json': {
'$ref': '#/components/schemas/SuccessEnvelope'}},
},
'default': {
'description': 'Error. Body is the error envelope; the code and '
'message are nested under data.error.',
'content': {'application/json': {
'$ref': '#/components/schemas/ErrorEnvelope'}},
},
}
# An endpoint that can be called without a token cannot answer 401.
if security and security != [{}]:
requires = not any(item == {} for item in security)
if requires:
responses['401'] = {'description': 'Missing or invalid credentials.'}
responses['403'] = {'description': 'Authenticated, but not permitted.'}
if parameters:
responses['404'] = {'description': 'No such record.'}
return responses
def _summary(text, fallback):
"""A summary that ends on a word.
Cutting at a fixed 120 characters truncated 95 of them mid-word, which is
what a tool picker shows a user as the whole description of the call.
"""
source = (text or fallback or '').strip()
if len(source) <= 120:
return source
head = source[:120]
cut = head.rfind(' ')
# Trailing connectives read worse than a clean cut: "servicelevel +..."
# promises a continuation the reader will never see.
return (head[:cut] if cut > 40 else head).rstrip(' ,;:.+-/&|(') + '...'
def build(surfaces):
paths = {}
tags = []
@@ -88,14 +195,21 @@ def build(surfaces):
desc.append('\n\n**Params:** ' + e['params'])
if e.get('example'):
desc.append('\n\n**Example:**\n```\n' + e['example'] + '\n```')
paths.setdefault(path, {})[verb] = {
security = _security(e.get('auth'))
parameters = _path_parameters(e.get('path'))
operation = {
'tags': [s['surface']],
'summary': (e.get('purpose') or path)[:120],
'summary': _summary(e.get('purpose'), path),
'description': ''.join(desc),
'security': _security(e.get('auth')),
'responses': {'200': {
'description': 'Success (success_response envelope)'}},
'security': security,
'responses': _responses(security, parameters),
}
if parameters:
operation['parameters'] = parameters
body = _request_body(verb, e)
if body:
operation['requestBody'] = body
paths.setdefault(path, {})[verb] = operation
return {
'openapi': '3.1.0',
'info': {
@@ -117,7 +231,61 @@ def build(surfaces):
# generator can honestly name; a site that wants its own in the spec
# sets SHOPDB_PUBLIC_URL when regenerating.
'servers': _servers(),
'components': {'securitySchemes': {
'components': {
'schemas': {
# The envelope every JSON endpoint returns. Worth spelling out
# because the error shape nests one level deeper than most
# people assume, and code written against the assumption reads
# undefined on every error it tries to report.
'SuccessEnvelope': {
'type': 'object',
'properties': {
'status': {'type': 'string', 'enum': ['success']},
'data': {'description': 'The payload. Shape is per endpoint.'},
'meta': {
'type': 'object',
'properties': {
'timestamp': {'type': 'string', 'format': 'date-time'},
'requestid': {'type': 'string'},
'pagination': {
'type': 'object',
'properties': {
'page': {'type': 'integer'},
'perpage': {'type': 'integer'},
'total': {'type': 'integer'},
'pages': {'type': 'integer'},
},
},
},
},
},
'required': ['status'],
},
'ErrorEnvelope': {
'type': 'object',
'properties': {
'status': {'type': 'string', 'enum': ['error']},
'data': {
'type': 'object',
'properties': {
'error': {
'type': 'object',
'properties': {
'code': {'type': 'string'},
'message': {'type': 'string'},
'details': {'type': 'object'},
},
'required': ['code', 'message'],
},
},
'required': ['error'],
},
'meta': {'type': 'object'},
},
'required': ['status', 'data'],
},
},
'securitySchemes': {
'bearerAuth': {'type': 'http', 'scheme': 'bearer', 'bearerFormat': 'JWT',
'description': 'Login token or a managed Personal Access Token (scoped).'},
'apiKeyAuth': {'type': 'apiKey', 'in': 'header', 'name': 'X-API-Key',

24
tests/test_docs_contract.py
View File

@@ -20,14 +20,24 @@ def test_hooks_doc_exists():
assert HOOKS_DOC.exists(), 'docs/PLUGIN-HOOKS.md is missing'
def test_hooks_doc_declares_current_contract_version():
"""The doc's version example must match the live __contract_version__."""
def test_hooks_doc_points_at_the_generated_version():
"""The doc must name the symbol and send the reader to the generated map.
This used to require the literal value in the page, which is what made it
stale everywhere else: nine documents copied a contract version and every
one of them was wrong, including a pin an external author would have failed
to load with. A doc that points at docs/PROJECT-MAP.md cannot go stale,
because the map is generated from shopdb/__init__.py.
See tests/test_docs_versions.py, which enforces the same rule the other way
round: no document may declare a version literal at all.
"""
text = HOOKS_DOC.read_text()
expected = f"__contract_version__ = '{__contract_version__}'"
assert expected in text, (
f'docs/PLUGIN-HOOKS.md version example is stale: expected {expected}. '
f'Update the "Contract version" section when bumping the contract.'
)
assert '__contract_version__' in text, (
'docs/PLUGIN-HOOKS.md should still name the symbol a plugin pins against.')
assert 'PROJECT-MAP.md' in text, (
'docs/PLUGIN-HOOKS.md should send the reader to the generated map for '
'the current value rather than restating it.')
def test_every_public_hook_is_documented():

123
tests/test_openapi_spec.py Normal file
View File

@@ -0,0 +1,123 @@
"""The generated spec has to be usable by a machine, not just readable.
docs/openapi.json is the only machine-readable description of this API, and it
is what the MCP server builds its tools from. It was published with no
`parameters` and no `requestBody` on any operation - invalid OpenAPI 3.1, and
worse in practice: a tool built from an operation with no parameters has nowhere
to put the id its caller supplied, so every argument was dropped in silence. A
call for one asset returned the list.
These tests are the floor. They check the spec's shape, not its prose.
"""
import json
import re
from pathlib import Path
import pytest
REPO = Path(__file__).resolve().parents[1]
SPEC = REPO / 'docs' / 'openapi.json'
pytestmark = pytest.mark.skipif(
not SPEC.is_file(),
reason='no generated spec in this checkout')
@pytest.fixture(scope='module')
def spec():
return json.loads(SPEC.read_text())
def operations(spec):
for path, item in spec['paths'].items():
for verb, operation in item.items():
yield path, verb, operation
def test_every_templated_path_declares_its_parameters(spec):
"""`/api/assets/{id}` with no parameters is not a description of anything."""
offenders = [
'%s %s' % (verb.upper(), path)
for path, verb, operation in operations(spec)
if '{' in path and not operation.get('parameters')
]
assert not offenders, (
'These operations have a templated path and no parameters, so a '
'generated client cannot fill it in:\n ' + '\n '.join(offenders[:15]))
def test_declared_parameters_match_the_template(spec):
"""A parameter that is not in the path, or a placeholder with no parameter,
is the same defect wearing different clothes."""
offenders = []
for path, verb, operation in operations(spec):
intemplate = set(re.findall(r'\{([^}]+)\}', path))
declared = {p['name'] for p in operation.get('parameters', [])
if p.get('in') == 'path'}
if intemplate != declared:
offenders.append('%s %s: template %s, declared %s'
% (verb.upper(), path, sorted(intemplate), sorted(declared)))
assert not offenders, '\n '.join(offenders[:15])
def test_write_verbs_can_carry_a_body(spec):
offenders = [
'%s %s' % (verb.upper(), path)
for path, verb, operation in operations(spec)
if verb in ('post', 'put') and 'requestBody' not in operation
]
assert not offenders, (
'These write operations declare no request body, so a client built '
'from this spec cannot send one:\n ' + '\n '.join(offenders[:15]))
def test_optional_auth_is_expressed_as_optional(spec):
"""An endpoint that works logged out must not be published as requiring a
token - that is a lie to every reader, and it is the majority case here."""
optional = [operation for _, _, operation in operations(spec)
if any(item == {} for item in operation.get('security') or [])]
assert len(optional) > 50, (
'The inventory documents over a hundred jwt-optional endpoints; the '
'spec should express them with an empty security requirement '
'alongside the scheme, not as bearer-required.')
def test_an_endpoint_that_needs_no_token_cannot_fail_with_401(spec):
offenders = [
'%s %s' % (verb.upper(), path)
for path, verb, operation in operations(spec)
if not operation.get('security') and '401' in operation.get('responses', {})
]
assert not offenders, '\n '.join(offenders[:15])
def test_every_operation_documents_failure(spec):
offenders = [
'%s %s' % (verb.upper(), path)
for path, verb, operation in operations(spec)
if 'default' not in operation.get('responses', {})
]
assert not offenders, (
'These operations document only success:\n ' + '\n '.join(offenders[:15]))
def test_the_envelopes_are_defined_and_referenced(spec):
schemas = spec.get('components', {}).get('schemas', {})
assert 'SuccessEnvelope' in schemas and 'ErrorEnvelope' in schemas
# The error shape is the one people get wrong: nested under data.
error = schemas['ErrorEnvelope']['properties']['data']['properties']['error']
assert set(error['required']) == {'code', 'message'}
def test_summaries_are_not_cut_mid_word(spec):
"""A truncated summary is what a tool picker shows as the whole description
of a call."""
offenders = []
for path, verb, operation in operations(spec):
summary = operation.get('summary', '')
if summary.endswith('...'):
body = summary[:-3]
if body and not body[-1].isalnum() and body[-1] not in ')]"\'':
offenders.append('%s %s: %r' % (verb.upper(), path, summary[-40:]))
assert not offenders, '\n '.join(offenders[:15])