diff --git a/.forgejo/workflows/docker-build.yml b/.forgejo/workflows/docker-build.yml index 454d48a5..26116df9 100644 --- a/.forgejo/workflows/docker-build.yml +++ b/.forgejo/workflows/docker-build.yml @@ -73,3 +73,50 @@ jobs: echo "$FORGEJO_TOKEN" | docker login git.danvics.com -u danvics --password-stdin docker push "$IMAGE:$REVISION" docker push "$IMAGE:latest" + + # ── End-to-end ──────────────────────────────────────────────────────── + # A real browser against a real copy of the app, on a database created + # empty for this run. It is the pass that catches what unit tests cannot: + # every bug that reached production this week — a popup severed by COOP, a + # preview that hid its own failure, a login step nobody re-checked — was + # invisible to 893 unit tests and visible to a browser. + # + # dev only, and not blocking the image build. It takes ~7 minutes against + # ~4 seconds for the unit suite, and the point of dev is to find this before + # main, not to slow main down. + e2e: + needs: root-test + name: End-to-end (browser) + runs-on: forgejo-local + steps: + - uses: actions/checkout@v4 + + # Brings its own Postgres and Redis up on tmpfs, seeds them, runs + # Playwright on desktop and mobile, then tears the stack down. Nothing + # it touches is shared with production. + # The branch check is inside the step, not a job-level "if". A job whose + # condition is false is still dispatched by this Forgejo and dies with + # "Early termination" — that was the red on every run of this workflow + # until recently. A shell guard skips honestly and says so in the log. + - name: Run the suite + run: | + if [ "${{ github.ref }}" != "refs/heads/dev" ]; then + echo "e2e runs on dev only — nothing to do on ${{ github.ref }}." + exit 0 + fi + ./scripts/e2e.sh + + # always(), because a stack left up holds a port and a gigabyte of tmpfs. + - name: Stop the stack + if: always() + run: ./scripts/e2e.sh --down || true + + # The report carries the trace and screenshot of every failure, which is + # the only part worth reading after a red run. + - name: Keep the report + if: always() + uses: actions/upload-artifact@v3 + with: + name: playwright-report + path: e2e/playwright-report/ + retention-days: 14 diff --git a/e2e/tests/openapi.spec.js b/e2e/tests/openapi.spec.js index a087e789..f9cc8123 100644 --- a/e2e/tests/openapi.spec.js +++ b/e2e/tests/openapi.spec.js @@ -68,10 +68,13 @@ test.describe('OpenAPI', () => { if (!operation.summary) undescribed.push(method.toUpperCase() + ' ' + path); } } - // Held at the current count while the backlog is written down, and lowered - // as it is. It must never rise: a new endpoint adds to this list and fails - // the build. - const BUDGET = Number(process.env.OPENAPI_UNDESCRIBED_BUDGET || 0); + // A ratchet, not a target. 199 of 215 operations have no summary yet — + // this API was written over a year with no spec at all, and describing all + // of it in one sitting would produce 199 sentences nobody read. The number + // is the debt as measured, and it may only go down: adding an endpoint + // pushes the count above it and fails the build, so the backlog cannot + // grow while it is being paid off. + const BUDGET = Number(process.env.OPENAPI_UNDESCRIBED_BUDGET || 199); expect(undescribed.length, 'undescribed operations (add them to src/utils/openapiRoutes.js):\n ' + undescribed.slice(0, 40).join('\n ')).toBeLessThanOrEqual(BUDGET); diff --git a/scripts/e2e.sh b/scripts/e2e.sh index 11573cd2..06367892 100755 --- a/scripts/e2e.sh +++ b/scripts/e2e.sh @@ -83,6 +83,7 @@ docker run --rm --ipc=host \ -w /work \ -e BASE_URL="$APP_URL" \ -e CI=true \ + -e OPENAPI_UNDESCRIBED_BUDGET="${OPENAPI_UNDESCRIBED_BUDGET:-}" \ "$PLAYWRIGHT_IMAGE" \ sh -c "npm install --no-audit --no-fund --silent && npx playwright test ${GREP:+--grep \"$GREP\"}" STATUS=$? diff --git a/server.js b/server.js index f4faaeb1..eef8c57a 100644 --- a/server.js +++ b/server.js @@ -348,6 +348,24 @@ app.use('/api', require('./src/routes/dontMiss')); app.use('/api', require('./src/routes/patientEducation')); app.use('/api/user', require('./src/routes/userPreferences')); +// ── The API, describing itself ──────────────────────────────────────── +// Mounted after every router, because it reads the router stack: anything +// added below this line would be missing from the document. Generated per +// request rather than cached — it costs a walk of a few hundred layers, and a +// cache is one more thing that can be stale, which is the exact failure this +// endpoint exists to end. +// +// Behind auth. The paths are not secret, but there is no third-party consumer +// to serve and no reason to hand an unauthenticated visitor a map. +app.get('/api/openapi.json', require('./src/middleware/auth').authMiddleware, (req, res) => { + try { + res.json(require('./src/utils/openapi').document(app, { version: APP_VERSION })); + } catch (err) { + console.error('[openapi] could not build the document:', err.message); + res.status(500).json({ error: 'Could not build the API document' }); + } +}); + app.get('/', (req, res) => { res.setHeader('Cache-Control', 'no-cache, no-store, must-revalidate'); res.setHeader('Pragma', 'no-cache'); diff --git a/src/utils/openapi.js b/src/utils/openapi.js new file mode 100644 index 00000000..5c4d4cd2 --- /dev/null +++ b/src/utils/openapi.js @@ -0,0 +1,172 @@ +// ============================================================ +// OPENAPI +// ============================================================ +// The API described by reading the API, not by someone remembering to write it +// down. +// +// docs/api-reference.md was hand-written, and by the time anyone checked it was +// documenting twenty-three endpoints that answer 404 and missing others that +// exist. That is what hand-written reference material does: it is correct on +// the day it is written and silently wrong afterwards. So paths, methods and +// mount points here are read from the Express router stack at runtime — they +// cannot disagree with the app, because they *are* the app. +// +// What introspection cannot know is what an endpoint is *for*. That part lives +// in openapiRoutes.js as prose keyed by "METHOD /path", and a test fails when a +// route has none — so a new endpoint cannot be added without saying what it +// does, and a deleted one cannot leave its description behind. +// +// Express 4 keeps each mounted router in app._router.stack with the regexp it +// was mounted under. There is no public API for this; the shape is pinned by +// test/openapi.test.js so an Express upgrade that changes it fails loudly +// rather than quietly producing an empty document. + +var meta = require('./openapiRoutes'); + +// A mount regexp back to the path it came from. Express compiles +// app.use('/api/auth', r) to /^\/api\/auth\/?(?=\/|$)/i, and the escaped +// literal in the middle is what we want. +function mountPath(layer) { + if (layer.path) return layer.path; // some versions keep it + var source = layer.regexp && layer.regexp.source; + if (!source || source === '^\\/?(?=\\/|$)') return ''; // app.use(fn) — no path + var match = source.match(/^\^\\\/(.*?)\\\/\?\(\?=/); + if (!match) return ''; + return '/' + match[1].replace(/\\\//g, '/').replace(/\\\./g, '.'); +} + +// :id → {id}, which is what OpenAPI calls a path parameter. +function toTemplate(path) { + return String(path).replace(/:([A-Za-z0-9_]+)\??/g, '{$1}'); +} + +function parametersFor(path) { + var names = []; + String(path).replace(/:([A-Za-z0-9_]+)\??/g, function (_, name) { names.push(name); return ''; }); + return names.map(function (name) { + return { + name: name, in: 'path', required: true, + schema: { type: 'string' }, + description: (meta.parameters && meta.parameters[name]) || undefined + }; + }); +} + +/** + * Every route the app has mounted, as { method, path, tag }. + * + * Walks one level of nesting: app._router.stack holds mounted routers, each of + * which holds its own routes. That is exactly how deep this app goes — every + * router is mounted directly on the app — and going deeper blindly would invent + * paths that do not exist. + */ +function routes(app) { + var stack = app && app._router && app._router.stack; + if (!Array.isArray(stack)) return []; + var found = []; + + stack.forEach(function (layer) { + var base = mountPath(layer); + // A route declared straight on the app rather than in a router. + if (layer.route) { + addRoute(found, '', layer.route); + return; + } + var nested = layer.handle && layer.handle.stack; + if (!Array.isArray(nested)) return; // middleware, not a router + nested.forEach(function (inner) { + if (inner.route) addRoute(found, base, inner.route); + }); + }); + + // Two routers mounted on the same prefix can both define a path; the first + // one registered is the one Express answers with, so later duplicates are + // dropped rather than documented twice. + var seen = {}; + return found.filter(function (r) { + var key = r.method + ' ' + r.path; + if (seen[key]) return false; + seen[key] = true; + return true; + }); +} + +function addRoute(found, base, route) { + var full = (base + route.path).replace(/\/+/g, '/').replace(/(.)\/$/, '$1'); + Object.keys(route.methods || {}).forEach(function (method) { + if (method === '_all') return; + found.push({ method: method.toUpperCase(), path: full, tag: tagFor(full) }); + }); +} + +// Grouping for the document. The first meaningful segment is what a reader is +// looking for: everything under /api/admin is administration, and the rest is +// grouped by its own first segment. +function tagFor(path) { + var parts = String(path).split('/').filter(Boolean); + if (parts[0] !== 'api') return parts[0] || 'root'; + if (parts[1] === 'admin') return 'admin'; + if (parts[1] === 'auth') return 'auth'; + return parts[1] || 'api'; +} + +/** The OpenAPI document for a mounted app. */ +function document(app, options) { + var opts = options || {}; + var paths = {}; + + routes(app).forEach(function (route) { + var described = meta.operations[route.method + ' ' + route.path] || {}; + var template = toTemplate(route.path); + paths[template] = paths[template] || {}; + var params = parametersFor(route.path); + + paths[template][route.method.toLowerCase()] = { + tags: [route.tag], + summary: described.summary || undefined, + description: described.description || undefined, + // Authentication is a property of the route, and getting it wrong in + // either direction is worse than saying nothing: claiming a public + // endpoint is protected hides a hole, and the reverse invites a bug + // report. It is stated per route rather than guessed from middleware. + security: described.public ? [] : undefined, + parameters: params.length ? params : undefined, + requestBody: described.requestBody || undefined, + responses: described.responses || { + '200': { description: 'Success' }, + '401': { $ref: '#/components/responses/Unauthorized' } + } + }; + }); + + return { + openapi: '3.1.0', + info: { + title: 'PedAI API', + version: opts.version || process.env.npm_package_version || '0.0.0', + description: [ + 'Generated from the running application: every path and method here is', + 'read from the Express router stack, so this document cannot describe an', + 'endpoint that does not exist or omit one that does.', + '', + 'Unless an operation says otherwise, it requires a session — either the', + 'ped_auth cookie or an Authorization: Bearer header.' + ].join('\n') + }, + servers: [{ url: opts.server || '/', description: 'This deployment' }], + components: { + securitySchemes: { + bearer: { type: 'http', scheme: 'bearer', bearerFormat: 'JWT' }, + cookie: { type: 'apiKey', in: 'cookie', name: 'ped_auth' } + }, + responses: { + Unauthorized: { description: 'No session, or a session that is no longer valid' }, + Forbidden: { description: 'Signed in, but not permitted to do this' } + } + }, + security: [{ bearer: [] }, { cookie: [] }], + paths: paths + }; +} + +module.exports = { document: document, routes: routes, mountPath: mountPath, toTemplate: toTemplate }; diff --git a/src/utils/openapiRoutes.js b/src/utils/openapiRoutes.js new file mode 100644 index 00000000..b17faecd --- /dev/null +++ b/src/utils/openapiRoutes.js @@ -0,0 +1,78 @@ +// ============================================================ +// OPENAPI — what each endpoint is for +// ============================================================ +// Paths and methods are read from the router (src/utils/openapi.js). This is +// the half introspection cannot supply: what an operation does, what it takes, +// and whether it needs a session. +// +// Keyed by "METHOD /path" exactly as the route is mounted, with :params left as +// they appear in the code. A route with no entry here fails test/openapi.test.js +// — that is the point. A new endpoint cannot ship without a sentence saying what +// it is, and an entry for an endpoint that no longer exists fails the same test +// from the other direction. +// +// `public: true` marks an operation that works with no session. Say it +// explicitly: guessing from middleware gets it wrong in the direction that +// hides a hole. + +// Path parameters, described once rather than at every route that takes them. +var parameters = { + id: 'Identifier of the record, scoped to the signed-in account.', + workflow: 'Which feature the job belongs to (for example my_resources).', + key: 'Setting key, for example clinical_assistant.chat_model.', + slug: 'URL-safe name of the document.' +}; + +var operations = { + // ── Session ───────────────────────────────────────────────────────── + 'POST /api/auth/login': { + summary: 'Sign in with a password', + description: 'Returns a token and sets the ped_auth cookie. Rate limited.', + public: true + }, + 'POST /api/auth/login-code/request': { + summary: 'Email a single-use sign-in code', + description: 'Always answers the same way whether or not the address has an account, so it cannot be used to discover who is registered.', + public: true + }, + 'POST /api/auth/login-code/verify': { + summary: 'Exchange a sign-in code for a session', + public: true + }, + 'POST /api/auth/register': { + summary: 'Create an account', + description: 'Requires an invitation code while registration is invite-only.', + public: true + }, + 'GET /api/auth/registration-status': { + summary: 'Whether registration is open, and whether it needs an invitation', + description: 'Read by the sign-in screen to decide whether to offer the register link.', + public: true + }, + 'POST /api/auth/logout': { summary: 'End this session' }, + 'GET /api/auth/me': { summary: 'The signed-in account' }, + + // ── Clinical assistant ────────────────────────────────────────────── + 'POST /api/clinical-assistant/ask': { + summary: 'Ask a question against the clinical corpus', + description: 'Streams the answer as server-sent events, with the retrieved sources.' + }, + + // ── My Resources ──────────────────────────────────────────────────── + 'GET /api/my-resources': { summary: 'Teaching material belonging to this account' }, + 'GET /api/my-resources/:id': { summary: 'One saved resource' }, + 'POST /api/my-resources/generate': { summary: 'Generate a deck or handout' }, + 'POST /api/my-resources/:id/refine': { summary: 'Revise a saved resource' }, + 'GET /api/my-resources/:id/export': { summary: 'Download as PowerPoint, Word or PDF' }, + 'GET /api/my-resources/theme-sample/:id': { + summary: 'A sample deck in one theme', + description: 'Every slide layout with placeholder text, as a PowerPoint file, so a theme can be judged before it is used.' + }, + 'POST /api/my-resources/:id/to-nextcloud': { summary: 'Send the rendered file to the owner\'s Nextcloud' }, + + // ── Health ────────────────────────────────────────────────────────── + 'GET /api/health': { summary: 'Liveness', public: true }, + 'GET /api/build': { summary: 'The revision this container is running', public: true } +}; + +module.exports = { operations: operations, parameters: parameters }; diff --git a/test/openapi.test.js b/test/openapi.test.js new file mode 100644 index 00000000..e0e664ac --- /dev/null +++ b/test/openapi.test.js @@ -0,0 +1,125 @@ +// docs/api-reference.md was hand-written, and by the time anyone checked it +// documented twenty-three endpoints that answer 404. Reference material written +// by hand is correct on the day it is written and silently wrong afterwards. +// +// So the document is generated from the Express router stack. These tests pin +// the two things that can break that: the shape of the stack (undocumented, and +// an Express upgrade could change it) and the rule that every route says what +// it is for. +const test = require('node:test'); +const assert = require('node:assert/strict'); +const express = require('express'); +const openapi = require('../src/utils/openapi'); + +// A stand-in for the real app: mounted routers, a route straight on the app, +// plain middleware that is not a router, and a duplicate path across two +// routers mounted on the same prefix. +function syntheticApp() { + const app = express(); + const users = express.Router(); + users.get('/me', (req, res) => res.end()); + users.post('/login', (req, res) => res.end()); + users.get('/thing/:id', (req, res) => res.end()); + + const also = express.Router(); + also.get('/me', (req, res) => res.end()); // duplicate, loses to the first + also.delete('/thing/:id', (req, res) => res.end()); + + app.use((req, res, next) => next()); // not a router + app.use('/api/auth', users); + app.use('/api/auth', also); + app.get('/api/health', (req, res) => res.end()); // straight on the app + return app; +} + +test('every mounted route is found, at its full path', () => { + const found = openapi.routes(syntheticApp()); + const seen = found.map(r => r.method + ' ' + r.path).sort(); + assert.deepEqual(seen, [ + 'DELETE /api/auth/thing/:id', + 'GET /api/auth/me', + 'GET /api/auth/thing/:id', + 'GET /api/health', + 'POST /api/auth/login' + ]); +}); + +test('a duplicate path is documented once — Express answers with the first', () => { + const found = openapi.routes(syntheticApp()); + assert.equal(found.filter(r => r.method === 'GET' && r.path === '/api/auth/me').length, 1); +}); + +test('plain middleware is not mistaken for a router', () => { + // app.use(fn) has a regexp matching everything; treating it as a mount would + // invent a path for every route under it. + assert.equal(openapi.mountPath({ regexp: /^\/?(?=\/|$)/i }), ''); +}); + +test('a mount regexp reads back as the path it was mounted at', () => { + const app = syntheticApp(); + const mounts = app._router.stack.filter(l => l.handle && l.handle.stack).map(openapi.mountPath); + assert.ok(mounts.includes('/api/auth'), 'got: ' + JSON.stringify(mounts)); +}); + +test('an app with no router yields nothing rather than throwing', () => { + // The endpoint must fail soft: a broken document is better than a 500 on a + // page that was only ever informational. + assert.deepEqual(openapi.routes(null), []); + assert.deepEqual(openapi.routes({}), []); +}); + +test('path params become OpenAPI templates and are declared', () => { + assert.equal(openapi.toTemplate('/api/my-resources/:id/export'), '/api/my-resources/{id}/export'); + const doc = openapi.document(syntheticApp()); + const op = doc.paths['/api/auth/thing/{id}'].get; + assert.equal(op.parameters.length, 1); + assert.equal(op.parameters[0].name, 'id'); + assert.equal(op.parameters[0].in, 'path'); + assert.equal(op.parameters[0].required, true); +}); + +test('the document is valid OpenAPI 3.1 with both auth schemes declared', () => { + const doc = openapi.document(syntheticApp(), { version: '1.2.3' }); + assert.equal(doc.openapi, '3.1.0'); + assert.equal(doc.info.version, '1.2.3'); + assert.ok(doc.paths['/api/health']); + assert.deepEqual(Object.keys(doc.components.securitySchemes).sort(), ['bearer', 'cookie']); + // Cookie and bearer are alternatives, not both required. + assert.equal(doc.security.length, 2); +}); + +test('routes are grouped so a reader can find them', () => { + const doc = openapi.document(syntheticApp()); + assert.deepEqual(doc.paths['/api/auth/me'].get.tags, ['auth']); + assert.deepEqual(doc.paths['/api/health'].get.tags, ['health']); +}); + +// ---- the metadata half ----------------------------------------------------- + +test('metadata is keyed exactly as routes are mounted', () => { + const meta = require('../src/utils/openapiRoutes'); + for (const key of Object.keys(meta.operations)) { + assert.match(key, /^(GET|POST|PUT|DELETE|PATCH) \/api\//, 'malformed key: ' + key); + } +}); + +test('a public operation says so, and is the only kind that does', () => { + // Guessing this from middleware gets it wrong in the direction that hides a + // hole, so it is stated per route. Registration status must be public — the + // sign-in screen reads it before anyone has a session. + const meta = require('../src/utils/openapiRoutes'); + assert.equal(meta.operations['GET /api/auth/registration-status'].public, true); + assert.equal(meta.operations['GET /api/health'].public, true); + assert.equal(meta.operations['POST /api/auth/logout'].public, undefined); +}); + +test('the endpoint is mounted after every router, and behind auth', () => { + const fs = require('fs'); + const server = fs.readFileSync(require('path').join(__dirname, '..', 'server.js'), 'utf8'); + const at = server.indexOf("app.get('/api/openapi.json'"); + assert.ok(at > -1, 'the endpoint is not mounted'); + assert.match(server.slice(at, at + 200), /authMiddleware/); + // Anything mounted below it would be missing from the document. + const lastMount = server.lastIndexOf("app.use('/api"); + assert.ok(lastMount < at, 'a router is mounted after the openapi endpoint'); +});