From 3add0063bfbeaf4f15f8ab490aa91b7a6899ef79 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Mon, 10 Aug 2026 06:35:22 -0500 Subject: [PATCH 01/30] feat(modules): installed_modules and the module state machine Phase 2 PR 1 of the module system (docs/website/MODULE_SYSTEM.md 2.7). The table and the state machine only: no loader, no routes, no boot wiring, so nothing an operator or a client can see changes and the route manifest diff is zero lines. The five states of 2.4 live in one `state` column: installed -> enabled -> started, with disabled and startup_failed as the recoverable ones. The row is a record of what happened, never the source of truth for what is mounted -- the loader scans the filesystem at require time, before the database is reachable (MODULE_API.md 4.1), which is what keeps routes.manifest.json generatable against a dead database. Two rules the model owns and the boot path will lean on: - Every boot resets each non-disabled row to `enabled` and clears its recorded failure, so a startup_failed module is retried on the next restart and a fixed one recovers with no admin-panel visit. `disabled` is the one operator decision rather than outcome, so it survives untouched -- and a disabled module's failure is a no-op, never a re-enable. - A failure carries the stage it happened at, and every non-failing transition clears it, so a running module can never show a stale reason. An illegal move throws instead of writing a row that misrepresents the state, except on the two boot-path softenings noted above, because one module's failure must never become everybody's. 22 model tests over an in-memory fake; the SQL and the DDL were round-tripped against a real MariaDB separately. Co-Authored-By: Claude --- server/db/schema.sql | 43 ++++ server/src/model/modules/modules.db.js | 59 +++++ server/src/model/modules/modules.model.js | 183 ++++++++++++++ server/test/modules.model.test.js | 295 ++++++++++++++++++++++ 4 files changed, 580 insertions(+) create mode 100644 server/src/model/modules/modules.db.js create mode 100644 server/src/model/modules/modules.model.js create mode 100644 server/test/modules.model.test.js diff --git a/server/db/schema.sql b/server/db/schema.sql index a61a8bf..2212c58 100644 --- a/server/db/schema.sql +++ b/server/db/schema.sql @@ -1318,6 +1318,49 @@ CREATE TABLE IF NOT EXISTS shard_atlas_pending ( CONSTRAINT chk_shard_atlas_pending_singleton CHECK (id = 1) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; +-- Installed modules (module system, docs/website/MODULE_SYSTEM.md §2.4). One row +-- per module the operator has installed onto the modules volume, keyed by the +-- module id from its module.json — the same id that names the directory, the URL +-- segment and the client registry key. +-- +-- This table is a RECORD of what happened, never the source of truth for what is +-- mounted: the loader scans the filesystem at require time, before the database is +-- reachable (MODULE_API.md §4.1), so the URL surface is a property of the volume +-- and not of a row here. What the row decides is whether a mounted module answers +-- (`disabled` ⇒ its guard 404s, §4.5) and what the admin panel shows after a +-- failure. +-- +-- `state` is the §2.4 machine in one column: installed → enabled → started, with +-- disabled and startup_failed as the recoverable states. `installed` is the +-- transient state between an install writing the row and the restart that starts +-- it. On every boot each non-disabled row is reset to `enabled` and re-attempted +-- (so a fixed module recovers on restart, with no panel visit needed), then the +-- load outcome writes `started` or `startup_failed`. Only `disabled` survives a +-- boot untouched — it is the operator's decision, not an outcome. +-- +-- failure_stage/failure_reason are §4.4's recorded reason, one of the seven +-- validation steps of §4.3 plus `boot`. Both are cleared by every transition that +-- is not a failure, so a stale reason can never be shown against a running module. +-- +-- source/sha256 are install provenance (§2.5): the release the bundle came from and +-- the digest that was verified before unpacking. Both NULL for a directory placed +-- on the volume by hand, which stays supported. +CREATE TABLE IF NOT EXISTS installed_modules ( + id VARCHAR(32) NOT NULL PRIMARY KEY, -- module.json id; names the directory + name VARCHAR(128) NOT NULL, -- human label for the admin Modules screen + version VARCHAR(32) NOT NULL, -- module.json version (semver) + state ENUM('installed','enabled','disabled','started','startup_failed') + NOT NULL DEFAULT 'installed', + failure_stage VARCHAR(32) NULL, -- manifest|core_api|mounts|extensions|schema|require|register|boot + failure_reason TEXT NULL, -- the recorded reason, shown in the admin panel + source VARCHAR(255) NULL, -- release URL the bundle came from + sha256 CHAR(64) NULL, -- verified bundle digest + installed_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + started_at DATETIME NULL, -- last successful start + updated_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, + INDEX idx_installed_modules_state (state) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + -- Migrations for databases created before the wiki upgrade. Each statement uses -- IF NOT EXISTS so re-running on every boot is a harmless no-op. New installs get -- these columns from the CREATE TABLE above; existing installs get them here. diff --git a/server/src/model/modules/modules.db.js b/server/src/model/modules/modules.db.js new file mode 100644 index 0000000..66474bb --- /dev/null +++ b/server/src/model/modules/modules.db.js @@ -0,0 +1,59 @@ +const { query } = require('../../utils/db') + +// SQL for installed_modules — the module system's record of what is installed and +// what happened to it on the last boot (db/schema.sql, docs/website/MODULE_SYSTEM.md +// §2.4). Rows are keyed by module id. All state rules live in modules.model.js; +// this file only moves rows. + +const COLS = `id, name, version, state, failure_stage, failure_reason, + source, sha256, installed_at, started_at, updated_at` + +const listAll = () => query(`SELECT ${COLS} FROM installed_modules ORDER BY id`) + +const getOne = (id) => query(`SELECT ${COLS} FROM installed_modules WHERE id = ?`, [id]) + +// Write (or refresh) the row for an installed module. A re-install or an upgrade +// updates the metadata and deliberately leaves `state` alone: upgrading an enabled +// module must not silently disable it, and re-installing a disabled one must not +// silently switch it back on. A brand-new row lands in `installed`, the transient +// state the next restart resolves. +const upsert = ({ id, name, version, source, sha256 }) => + query( + `INSERT INTO installed_modules (id, name, version, source, sha256, state) + VALUES (?, ?, ?, ?, ?, 'installed') + ON DUPLICATE KEY UPDATE + name = VALUES(name), + version = VALUES(version), + source = VALUES(source), + sha256 = VALUES(sha256)`, + [id, name, version, source ?? null, sha256 ?? null], + ) + +// Move one row to a new state. `failureStage`/`failureReason` are written on every +// call — a non-failing transition passes nulls, which is what clears a stale reason +// off a module that has since come up. `stampStarted` sets started_at to now. +const setState = ({ id, state, failureStage = null, failureReason = null, stampStarted = false }) => + query( + `UPDATE installed_modules + SET state = ?, failure_stage = ?, failure_reason = ? + ${stampStarted ? ', started_at = CURRENT_TIMESTAMP' : ''} + WHERE id = ?`, + [state, failureStage, failureReason, id], + ) + +// Boot reset: every row the operator has not disabled goes back to `enabled` with +// no failure recorded, so the load that follows writes this boot's outcome rather +// than leaving the last one on display. `disabled` is untouched — it is a decision, +// not an outcome. +const resetForBoot = () => + query( + `UPDATE installed_modules + SET state = 'enabled', failure_stage = NULL, failure_reason = NULL + WHERE state <> 'disabled'`, + ) + +// Drop the row entirely. Only the explicit purge does this (§2.5); a plain +// uninstall disables the module and keeps its row and its data. +const remove = (id) => query('DELETE FROM installed_modules WHERE id = ?', [id]) + +module.exports = { listAll, getOne, upsert, setState, resetForBoot, remove } diff --git a/server/src/model/modules/modules.model.js b/server/src/model/modules/modules.model.js new file mode 100644 index 0000000..3ad7223 --- /dev/null +++ b/server/src/model/modules/modules.model.js @@ -0,0 +1,183 @@ +// The module state machine (docs/website/MODULE_SYSTEM.md §2.4, MODULE_API.md §4.4). +// +// installed ──► enabled ──► started +// │ │ +// │ └──► startup_failed ──┐ +// │ │ (retry) +// └──────────────► disabled ◄─────────┘ +// +// One row per installed module, one column holding the state. The rules that make +// the machine mean anything live here, not in the SQL: +// +// - `installed` is transient. An install writes the row; the restart that follows +// resolves it to `started` or `startup_failed` (§2.5). +// - `disabled` is the only state a boot leaves alone. It is the operator's +// decision; every other state is an outcome and is recomputed each boot by +// beginBoot(). That is what makes a fixed module recover on restart without +// anyone visiting the admin panel. +// - A failure is recorded with the stage it happened at, and every non-failing +// transition clears it — a running module can never show a stale reason. +// +// What this table does NOT decide is which routes exist. The loader scans the +// filesystem at require time, before the database is reachable (MODULE_API.md §4.1), +// so a disabled module is still mounted and simply guarded (§4.5). Keeping the URL +// surface a property of the volume is what lets routes.manifest.json be generated +// off a dead database. + +const db = require('./modules.db') + +const STATES = ['installed', 'enabled', 'disabled', 'started', 'startup_failed'] + +// The stage a failure happened at: MODULE_API.md §4.3's seven validation steps, +// plus `boot` for an onBoot hook that threw (§2.5). +const FAILURE_STAGES = [ + 'manifest', + 'core_api', + 'mounts', + 'extensions', + 'schema', + 'require', + 'register', + 'boot', +] + +class ModuleStateError extends Error { + constructor(code, message) { + super(message) + this.name = 'ModuleStateError' + this.code = code + } +} + +// Legal moves, keyed by target state. Anything not listed is a bug in the caller +// and throws rather than writing a row that misrepresents what happened. +const ALLOWED_FROM = { + // Enabling is the recovery path as well as the first step: a disabled module the + // operator switches back on, and a startup_failed one they retry, both land here. + enabled: ['installed', 'enabled', 'disabled', 'startup_failed', 'started'], + // The operator may disable a module in any state, including one that is running. + disabled: STATES, + // Reached from `enabled` on a normal boot, and from `installed` on the first boot + // after an install (or for a directory placed on the volume by hand, whose row is + // written moments earlier in the same boot). + started: ['installed', 'enabled'], + // Failure always precedes `started` in the lifecycle; `started` is accepted so a + // late failure can still be recorded truthfully rather than dropped. + startup_failed: ['installed', 'enabled', 'started'], +} + +// row → API shape. +function serialize(row) { + if (!row) return null + return { + id: row.id, + name: row.name, + version: row.version, + state: row.state, + failureStage: row.failure_stage ?? null, + failureReason: row.failure_reason ?? null, + source: row.source ?? null, + sha256: row.sha256 ?? null, + installedAt: row.installed_at ?? null, + startedAt: row.started_at ?? null, + updatedAt: row.updated_at ?? null, + } +} + +async function list() { + const rows = await db.listAll() + return rows.map(serialize) +} + +async function get(id) { + const rows = await db.getOne(id) + return serialize(rows[0]) +} + +// Record an install (or a re-install / upgrade). Metadata is refreshed; the state is +// left as it is, so upgrading an enabled module does not switch it off and +// re-installing a disabled one does not switch it on. A new row lands in `installed`. +async function recordInstalled({ id, name, version, source = null, sha256 = null }) { + if (!id || !name || !version) { + throw new ModuleStateError('invalid_module', 'id, name and version are required') + } + await db.upsert({ id, name, version, source, sha256 }) + return get(id) +} + +// Start of boot: clear the last boot's outcomes so what is on display after this +// boot is what this boot did. Leaves `disabled` rows alone (see the header). +// Returns the number of rows reset. +async function beginBoot() { + const res = await db.resetForBoot() + return res?.affectedRows ?? 0 +} + +// Apply one transition, after checking it is legal for the row's current state. +// A row that does not exist is not an error the caller can act on — a module can be +// present on the volume with no row at all — so it returns null and writes nothing. +async function transition(id, target, { failureStage = null, failureReason = null } = {}) { + const current = await get(id) + if (!current) return null + + const allowed = ALLOWED_FROM[target] + if (!allowed.includes(current.state)) { + throw new ModuleStateError( + 'illegal_transition', + `module '${id}': cannot move from '${current.state}' to '${target}'`, + ) + } + + await db.setState({ + id, + state: target, + failureStage, + failureReason, + stampStarted: target === 'started', + }) + return get(id) +} + +const enable = (id) => transition(id, 'enabled') +const disable = (id) => transition(id, 'disabled') +const markStarted = (id) => transition(id, 'started') + +// Record a failure at a named stage. Two deliberate softenings, both because this is +// called from the boot path where throwing would turn one module's failure into +// everybody's (MODULE_API.md §4.4 — the failing module fails alone): +// +// - a `disabled` row is a no-op. The operator switched it off; a broken module +// they already disabled is not news, and overwriting their decision with an +// outcome would silently re-enable it on the next boot. +// - an unrecognised stage is recorded as `require` rather than rejected, so a +// miscategorised failure still reaches the admin panel with its reason intact. +async function markStartupFailed(id, { stage, reason }) { + const current = await get(id) + if (!current || current.state === 'disabled') return current + + return transition(id, 'startup_failed', { + failureStage: FAILURE_STAGES.includes(stage) ? stage : 'require', + failureReason: String(reason ?? 'unknown error').slice(0, 4000), + }) +} + +// Purge only (§2.5). A plain uninstall disables the module and keeps its row, so its +// data survives and the admin panel can still show what was there. +async function remove(id) { + await db.remove(id) +} + +module.exports = { + STATES, + FAILURE_STAGES, + ModuleStateError, + list, + get, + recordInstalled, + beginBoot, + enable, + disable, + markStarted, + markStartupFailed, + remove, +} diff --git a/server/test/modules.model.test.js b/server/test/modules.model.test.js new file mode 100644 index 0000000..4147acc --- /dev/null +++ b/server/test/modules.model.test.js @@ -0,0 +1,295 @@ +// Point the DB pool at a dead port before it's built; every modules.db method is +// monkeypatched below, and pool.close() at the end lets the process exit cleanly. +process.env.DB_HOST = '127.0.0.1' +process.env.DB_PORT = '59999' + +const { test, beforeEach, after } = require('node:test') +const assert = require('node:assert/strict') +const pool = require('../src/utils/db') +after(() => pool.close()) + +// Unit-test the module state machine (docs/website/MODULE_SYSTEM.md §2.4) against an +// in-memory fake by monkeypatching modules.db (no DB). What is locked here is +// everything the boot path and the admin panel will lean on in later phases: +// - `installed` is transient and a re-install never overwrites the operator's +// enable/disable decision; +// - beginBoot() recomputes outcomes and leaves `disabled` alone — the property that +// makes a fixed module recover on a restart with no panel visit; +// - a failure carries its stage and reason, and every non-failing transition clears +// them, so a running module can never display a stale reason; +// - a disabled module's failure is a no-op, because the boot path must never turn +// one module's failure into a re-enable of a module the operator switched off; +// - an illegal move throws instead of writing a row that misrepresents the state. +const modulesDb = require('../src/model/modules/modules.db') +const modules = require('../src/model/modules/modules.model') + +let rows // id → row, snake_case exactly as modules.db returns it + +const saved = { ...modulesDb } + +function reset() { + rows = new Map() +} + +modulesDb.listAll = async () => [...rows.values()].sort((a, b) => a.id.localeCompare(b.id)) + +modulesDb.getOne = async (id) => (rows.has(id) ? [rows.get(id)] : []) + +modulesDb.upsert = async ({ id, name, version, source, sha256 }) => { + const existing = rows.get(id) + if (existing) { + Object.assign(existing, { name, version, source: source ?? null, sha256: sha256 ?? null }) + return { affectedRows: 1 } + } + rows.set(id, { + id, + name, + version, + state: 'installed', + failure_stage: null, + failure_reason: null, + source: source ?? null, + sha256: sha256 ?? null, + installed_at: '2026-08-10T00:00:00Z', + started_at: null, + updated_at: '2026-08-10T00:00:00Z', + }) + return { affectedRows: 1 } +} + +modulesDb.setState = async ({ id, state, failureStage, failureReason, stampStarted }) => { + const row = rows.get(id) + if (!row) return { affectedRows: 0 } + row.state = state + row.failure_stage = failureStage ?? null + row.failure_reason = failureReason ?? null + if (stampStarted) row.started_at = '2026-08-10T12:00:00Z' + return { affectedRows: 1 } +} + +modulesDb.resetForBoot = async () => { + let n = 0 + for (const row of rows.values()) { + if (row.state === 'disabled') continue + row.state = 'enabled' + row.failure_stage = null + row.failure_reason = null + n += 1 + } + return { affectedRows: n } +} + +modulesDb.remove = async (id) => ({ affectedRows: rows.delete(id) ? 1 : 0 }) + +after(() => Object.assign(modulesDb, saved)) + +beforeEach(reset) + +// Install one module and put it in a given state, bypassing the machine so a test +// can start from any state without asserting its way there. +async function seed(id, state = 'installed', extra = {}) { + await modules.recordInstalled({ id, name: `Module ${id}`, version: '1.0.0', ...extra }) + rows.get(id).state = state + return modules.get(id) +} + +// ── recordInstalled ─────────────────────────────────────────────────── + +test('a new install lands in the transient installed state', async () => { + const mod = await modules.recordInstalled({ + id: 'uo', + name: 'Ultima Online', + version: '1.2.0', + source: 'https://gitea.example/releases/module-uo-1.2.0.tar.gz', + sha256: 'a'.repeat(64), + }) + assert.equal(mod.state, 'installed') + assert.equal(mod.version, '1.2.0') + assert.equal(mod.sha256, 'a'.repeat(64)) + assert.equal(mod.startedAt, null) + assert.equal(mod.failureReason, null) +}) + +test('a hand-placed module records with no provenance', async () => { + const mod = await modules.recordInstalled({ id: 'uo', name: 'Ultima Online', version: '1.0.0' }) + assert.equal(mod.source, null) + assert.equal(mod.sha256, null) +}) + +test('recordInstalled refuses a manifest missing id, name or version', async () => { + await assert.rejects( + () => modules.recordInstalled({ id: 'uo', version: '1.0.0' }), + (err) => err.code === 'invalid_module', + ) +}) + +test('an upgrade refreshes metadata and leaves the operator decision alone', async () => { + await seed('uo', 'disabled') + const mod = await modules.recordInstalled({ id: 'uo', name: 'Ultima Online', version: '2.0.0' }) + assert.equal(mod.version, '2.0.0') + assert.equal(mod.name, 'Ultima Online') + assert.equal(mod.state, 'disabled', 're-installing must not silently re-enable') +}) + +test('an upgrade of a started module does not switch it off', async () => { + await seed('uo', 'started') + const mod = await modules.recordInstalled({ id: 'uo', name: 'Module uo', version: '1.1.0' }) + assert.equal(mod.state, 'started') +}) + +// ── the machine ─────────────────────────────────────────────────────── + +test('installed → enabled → started, stamping the start', async () => { + await seed('uo') + assert.equal((await modules.enable('uo')).state, 'enabled') + const started = await modules.markStarted('uo') + assert.equal(started.state, 'started') + assert.ok(started.startedAt, 'a successful start is stamped') +}) + +test('a first boot may start a module straight from installed', async () => { + await seed('uo') + assert.equal((await modules.markStarted('uo')).state, 'started') +}) + +test('a disabled module can be re-enabled', async () => { + await seed('uo', 'disabled') + assert.equal((await modules.enable('uo')).state, 'enabled') +}) + +test('a failed module is retried by enabling it, which clears the reason', async () => { + await seed('uo', 'enabled') + await modules.markStartupFailed('uo', { stage: 'boot', reason: 'atlas refresh threw' }) + const retried = await modules.enable('uo') + assert.equal(retried.state, 'enabled') + assert.equal(retried.failureStage, null) + assert.equal(retried.failureReason, null) +}) + +test('a running module can be disabled', async () => { + await seed('uo', 'started') + assert.equal((await modules.disable('uo')).state, 'disabled') +}) + +test('a disabled module is never started — that would be a core bug, so it throws', async () => { + await seed('uo', 'disabled') + await assert.rejects( + () => modules.markStarted('uo'), + (err) => err.name === 'ModuleStateError' && err.code === 'illegal_transition', + ) + assert.equal((await modules.get('uo')).state, 'disabled') +}) + +test('a transition on a module with no row writes nothing and returns null', async () => { + assert.equal(await modules.enable('ghost'), null) + assert.equal(await modules.markStarted('ghost'), null) + assert.equal(rows.size, 0) +}) + +// ── failures ────────────────────────────────────────────────────────── + +test('a failure records its stage and reason', async () => { + await seed('uo', 'enabled') + const failed = await modules.markStartupFailed('uo', { + stage: 'core_api', + reason: "module 'uo' needs coreApi ^2.0.0, core provides 1.0.0", + }) + assert.equal(failed.state, 'startup_failed') + assert.equal(failed.failureStage, 'core_api') + assert.match(failed.failureReason, /coreApi/) +}) + +test('an unrecognised stage is still recorded, as require', async () => { + await seed('uo', 'enabled') + const failed = await modules.markStartupFailed('uo', { stage: 'nonsense', reason: 'boom' }) + assert.equal(failed.failureStage, 'require') + assert.equal(failed.failureReason, 'boom') +}) + +test('a missing reason still produces a displayable one', async () => { + await seed('uo', 'enabled') + const failed = await modules.markStartupFailed('uo', { stage: 'register' }) + assert.equal(failed.failureReason, 'unknown error') +}) + +test('a runaway reason is truncated rather than refused', async () => { + await seed('uo', 'enabled') + const failed = await modules.markStartupFailed('uo', { stage: 'boot', reason: 'x'.repeat(9000) }) + assert.equal(failed.failureReason.length, 4000) +}) + +test("a disabled module's failure is a no-op, not a re-enable", async () => { + await seed('uo', 'disabled') + const unchanged = await modules.markStartupFailed('uo', { stage: 'require', reason: 'broken' }) + assert.equal(unchanged.state, 'disabled') + assert.equal(unchanged.failureReason, null) +}) + +test('starting successfully clears the previous failure', async () => { + await seed('uo', 'enabled') + await modules.markStartupFailed('uo', { stage: 'schema', reason: 'bad fragment' }) + await modules.enable('uo') + const started = await modules.markStarted('uo') + assert.equal(started.failureStage, null) + assert.equal(started.failureReason, null) +}) + +// ── boot ────────────────────────────────────────────────────────────── + +test('beginBoot recomputes outcomes and leaves disabled alone', async () => { + await seed('a', 'started') + await seed('b', 'startup_failed') + await seed('c', 'disabled') + await seed('d', 'installed') + rows.get('b').failure_reason = 'last boot blew up' + rows.get('b').failure_stage = 'boot' + + assert.equal(await modules.beginBoot(), 3) + + const byId = Object.fromEntries((await modules.list()).map((m) => [m.id, m])) + assert.equal(byId.a.state, 'enabled') + assert.equal(byId.b.state, 'enabled', 'a failed module is retried on the next boot') + assert.equal(byId.b.failureReason, null, 'and last boot’s reason is cleared') + assert.equal(byId.c.state, 'disabled', 'the operator decision survives a boot') + assert.equal(byId.d.state, 'enabled') +}) + +test('beginBoot keeps the start stamp of a module that was running', async () => { + await seed('uo', 'enabled') + await modules.markStarted('uo') + await modules.beginBoot() + assert.ok((await modules.get('uo')).startedAt, 'started_at is the last successful start') +}) + +// ── list / purge ────────────────────────────────────────────────────── + +test('list returns every module, id-ordered and serialized', async () => { + await seed('zzz') + await seed('aaa') + const all = await modules.list() + assert.deepEqual( + all.map((m) => m.id), + ['aaa', 'zzz'], + ) + assert.deepEqual(Object.keys(all[0]).sort(), [ + 'failureReason', + 'failureStage', + 'id', + 'installedAt', + 'name', + 'sha256', + 'source', + 'startedAt', + 'state', + 'updatedAt', + 'version', + ]) +}) + +test('purge drops the row; uninstall is a disable and keeps it', async () => { + await seed('uo', 'started') + await modules.disable('uo') + assert.ok(await modules.get('uo'), 'uninstall keeps the row and its data') + await modules.remove('uo') + assert.equal(await modules.get('uo'), null) +}) -- 2.49.1 From ec1ca7e79433380b9d9e515b8438a0beb32d8cbb Mon Sep 17 00:00:00 2001 From: wtclaude Date: Mon, 10 Aug 2026 14:35:27 -0500 Subject: [PATCH 02/30] feat(modules): the filesystem module loader Phase 2 PR 2 of docs/website/MODULE_SYSTEM.md 2.7. Adds server/src/modules/{loader,semver,version}.js: the synchronous scan of MODULES_DIR, manifest validation, prefix and table-name collision rejection, per-module try/catch and the mount into the three tier routers behind the MODULE_API.md 4.5 dispatch guard. Two decisions the contract left open, both now written up there: - The load trigger is one explicit modules.load(tierRouters) call in app.js, not a lazy scan (API 7.6). Accessors throw until it has run, because "no modules installed" is a real answer a caller must not be handed by accident. - Whether core owns a prefix is asked of the live tier routers via express's own layer.match(), skipping root-mounted layers, rather than a hardcoded table -- the spike's was already stale when written (API 4.3). Mounting is a second pass after every module is validated. Doing it inside the scan loop makes the first module's layers indistinguishable from core's, so the second module claiming a taken prefix is told it collided with core and the module-versus-module check is unreachable. registerExtension/NotificationStreams/AnnounceLeg and onBoot/onShutdown throw "not available until phase 2 PR 4/5" rather than no-op; an accepting stub would let a module believe it had registered something. No schema replay, no boot dispatch, no installed_modules reconcile -- those are PRs 3 and 5, and until PR 5 a record's state is in memory only. No module ships on the volume, so nothing an operator or client can see changes: 842 tests pass, routes.manifest.json is unchanged at 229 routes and swagger-output.json regenerates byte-identical. Co-Authored-By: Claude --- server/src/app.js | 23 ++ server/src/modules/loader.js | 473 +++++++++++++++++++++++++++++++ server/src/modules/semver.js | 47 +++ server/src/modules/version.js | 14 + server/test/moduleLoader.test.js | 444 +++++++++++++++++++++++++++++ 5 files changed, 1001 insertions(+) create mode 100644 server/src/modules/loader.js create mode 100644 server/src/modules/semver.js create mode 100644 server/src/modules/version.js create mode 100644 server/test/moduleLoader.test.js diff --git a/server/src/app.js b/server/src/app.js index 62bfe38..0f21a1e 100644 --- a/server/src/app.js +++ b/server/src/app.js @@ -10,6 +10,7 @@ require('dotenv').config() const swaggerUi = require('swagger-ui-express') const apiRouter = require('./router/api.router') +const modules = require('./modules/loader') const wellKnown = require('./router/wellKnown.controller') const cspReport = require('./router/cspReport.controller') const brand = require('./config/brand') @@ -154,6 +155,28 @@ app.get( app.post(csp.REPORT_PATH, cspReportLimiter, ...cspReport.parsers, cspReport.receive) app.use('/api', apiRouter) + +// ── Installed modules ───────────────────────────────────────────────── +// Discover, validate and mount whatever is on the modules volume +// (docs/website/MODULE_API.md Part 4). One explicit call, here and nowhere else: +// the loader has no lazy self-scan, so there is exactly one place that decides +// when modules are discovered, and reading the module list before this line is +// an error rather than a silent empty answer (§7.6). +// +// Position is load-bearing, in both directions. It is AFTER `/api` is mounted, +// so every core prefix is already on the tier routers when the collision check +// asks them what core owns — and so first-match-wins means a module physically +// cannot shadow a core route. It is BEFORE the `/api` 404 below, so a module +// route reaches its handler instead of the catch-all. +// +// The three requires resolve from cache to the very routers v1.router.js +// mounted; this is a reference to them, not a second copy. +modules.load({ + public: require('./router/v1/public'), + admin: require('./router/v1/admin'), + player: require('./router/v1/player'), +}) + app.use('/api', (req, res) => res.status(404).json({ message: 'Not found' })) // ── /.well-known ────────────────────────────────────────────────────── diff --git a/server/src/modules/loader.js b/server/src/modules/loader.js new file mode 100644 index 0000000..0fc7e6f --- /dev/null +++ b/server/src/modules/loader.js @@ -0,0 +1,473 @@ +// ── The module loader ────────────────────────────────────────────────────── +// +// Phase 2, PR 2 of docs/website/MODULE_SYSTEM.md §2.7. The normative contract is +// docs/website/MODULE_API.md Part 4; where the two disagree, the contract wins. +// +// The one property this file exists to guarantee, and the reason it looks the +// way it does: +// +// **The filesystem is the mounting source of truth, and mounting is +// SYNCHRONOUS.** `scripts/routeManifest.js:38` and `swagger/swagger.js:29` +// both require app.js with the pool pointed at a dead port. A loader that +// awaited a database row before mounting would make every module route +// invisible to the frozen-URL-surface test (§1.12). So: readdirSync at require +// time, no database, no promises (§4.1). +// +// A module that fails ANYWHERE in this file fails alone. Nothing here may throw +// past its own try/catch — a bad module must cost the site its routes, never its +// boot (§4.4). +// +// What is deliberately NOT here yet, each landing with the PR that first calls +// it (§2.7): schema-fragment replay (PR 3), the three de-entanglement registries +// (PR 4), boot/shutdown hook dispatch and the `installed_modules` reconcile +// (PR 5), GET /api/v1/public/modules and the client chunk's static mount +// (PRs 6-7). Until PR 5 the state a module carries is in memory only. + +const fs = require('fs') +const path = require('path') + +const { MODULE_API_VERSION } = require('./version') +const semver = require('./semver') + +const log = require('../utils/logger')('modules') + +const REPO_ROOT = path.join(__dirname, '..', '..', '..') +const MODULES_DIR = process.env.MODULES_DIR || path.join(REPO_ROOT, 'modules') + +// One segment, lowercase, no parameters. A module prefix that could contain a +// `/` or a `:` would let a module reach outside the slot it was given. +const ID = /^[a-z][a-z0-9-]{1,31}$/ +const PREFIX = /^\/[a-z0-9][a-z0-9-]*$/ +const TIERS = ['public', 'admin', 'player'] + +const MANIFEST_KEYS = new Set([ + 'id', 'name', 'version', 'coreApi', 'server', 'client', + 'schema', 'purge', 'mounts', 'extensions', 'capabilities', +]) + +// Extension slots core declares (§2.4). Only core may declare one; a module may +// only fill one. Validation rejects a manifest naming a slot that does not +// exist — `registerExtension` itself arrives with PR 4. +const CORE_SLOTS = new Set(['admin.users.detail']) + +// id → record. Populated by load(), read by list(). +const modules = new Map() +let loaded = false + +// ── ctx ──────────────────────────────────────────────────────────────────── + +// Everything a module may reach in core, and nothing else (§2.3). Required +// lazily inside the factory rather than at file scope: this file is required by +// app.js, and hoisting these to the top would make the DB pool, the settings +// model and the upload directory startup-time dependencies of the loader itself. +function buildCtx(id, moduleRoot) { + /* eslint-disable global-require */ + // The shared SERVER dependencies — the exact counterpart of window.__rg's + // react/react-dom/react-router on the client, and load-bearing for the same + // two reasons (§7.2). + // + // 1. A module lives at /modules//, OUTSIDE server/, so Node's + // resolver walks up from there and never sees server/node_modules. A + // module that required 'express' itself would fail to load — which is + // exactly how this was discovered. + // 2. Even if it resolved, a second copy of express in the process is a + // second Router prototype and a second set of instanceof checks. One + // express, owned by core, is the same rule as one React. + // + // The consequence for a module author is the same on both sides: declare these + // external, never bundle them, take them from what core hands you. + const express = require('express') + const validator = require('express-validator') + const db = require('../utils/db') + const settings = require('../model/settings/settings.model') + const posts = require('../model/posts/posts.model') + const auth = require('../utils/auth') + const pushDispatch = require('../utils/pushDispatch') + const secretBox = require('../utils/secretBox') + const createLogger = require('../utils/logger') + const { requireAuth, requireRole } = require('../auth/session.middleware') + const siteMode = require('../middleware/siteMode') + const validate = require('../middleware/validate') + const noindex = require('../middleware/noindex') + const uploads = require('../router/v1/admin/imageUpload') + /* eslint-enable global-require */ + + // Narrowed on purpose (§2.3): utils/auth also re-exports signToken, + // setAuthCookie and the TOTP challenge primitives, and minting a session is + // core's job. A module that needs an identity needs to READ one. + const ctx = { + moduleId: id, + paths: { moduleRoot }, + express, + validator, + db: { query: db.query, pool: db.pool }, + log: (namespace) => createLogger(namespace ? `${id}:${namespace}` : id), + settings: { + get: settings.get, + set: settings.set, + getInstanceName: settings.getInstanceName, + }, + auth: { getUserFromRequest: auth.getUserFromRequest }, + push: { publish: pushDispatch.publish }, + secretBox: { encrypt: secretBox.encrypt, decrypt: secretBox.decrypt }, + middleware: { requireAuth, requireRole, siteMode, validate, noindex }, + uploads, + posts: { + listAll: posts.listAll, + getById: posts.getById, + linkAnnounceJob: posts.linkAnnounceJob, + markAnnounced: posts.markAnnounced, + }, + } + // A guard against accident, not against a hostile module — the boundary is + // organisational, not a security boundary (MODULE_SYSTEM.md §2.2). + for (const value of Object.values(ctx)) { + if (value && typeof value === 'object') Object.freeze(value) + } + return Object.freeze(ctx) +} + +// ── The registration api ─────────────────────────────────────────────────── + +// Collects what the module registers so validation can compare it against what +// module.json DECLARED. Declaration is the contract; a module that registers a +// prefix it did not declare is rejected, because module.json is what the admin +// panel, the collision check and the reviewer all read. +function buildApi(record) { + const once = (name) => { + if (record.called.has(name)) throw new Error(`${name}() called twice`) + record.called.add(name) + } + // PR 4 brings the three de-entanglement registries and PR 5 the boot hooks. + // They throw rather than no-op: an accepting stub would let a module believe + // it had registered something and fail silently at the far end. + const notYet = (name, pr) => () => { + throw new Error(`${name}: not available until phase 2 PR ${pr}`) + } + return { + registerRoutes(mounts) { + once('registerRoutes') + if (!mounts || typeof mounts !== 'object') throw new Error('registerRoutes: expected an object') + for (const [tier, byPrefix] of Object.entries(mounts)) { + if (!TIERS.includes(tier)) throw new Error(`registerRoutes: unknown tier "${tier}"`) + for (const [prefix, router] of Object.entries(byPrefix)) { + if (!PREFIX.test(prefix)) throw new Error(`registerRoutes: bad prefix "${prefix}"`) + if (typeof router !== 'function') throw new Error(`registerRoutes: ${tier}${prefix} is not a router`) + record.routes[tier].set(prefix, router) + } + } + }, + registerExtension: notYet('registerExtension', 4), + registerNotificationStreams: notYet('registerNotificationStreams', 4), + registerAnnounceLeg: notYet('registerAnnounceLeg', 4), + onBoot: notYet('onBoot', 5), + onShutdown: notYet('onShutdown', 5), + } +} + +// ── Validation ───────────────────────────────────────────────────────────── + +// Table names a module may create despite not carrying its own id as a prefix. +// +// module-uo's twenty-seven tables predate the module system by two years, and +// renaming live tables is a data migration this workstream deliberately does not +// do (MODULE_SYSTEM.md §1.6). Grandfathering them by an explicit, per-module +// allowlist keeps the prefix rule real for every module written after this one — +// the alternative, dropping the rule, would leave the first name collision to be +// discovered by a module silently adopting someone else's table. +const LEGACY_TABLE_PREFIXES = { uo: ['shard_', 'uo_link_'] } + +const CREATE_TABLE = /CREATE\s+TABLE\s+(?:IF\s+NOT\s+EXISTS\s+)?[`"]?(\w+)[`"]?/gi + +/** Table names core's own schema.sql declares — a module may not touch these. */ +let coreTables = null +function coreTableNames() { + if (coreTables) return coreTables + coreTables = new Set() + try { + const sql = fs.readFileSync(path.join(__dirname, '..', '..', 'db', 'schema.sql'), 'utf8') + for (const m of sql.matchAll(CREATE_TABLE)) coreTables.add(m[1].toLowerCase()) + } catch (err) { + log.warn('could not read core schema for the table-collision check', { message: err.message }) + } + return coreTables +} + +/** Every table name a schema fragment declares. Throws if the file is unreadable. */ +function tablesOf(dir, manifest) { + if (!manifest.schema) return new Set() + const sql = fs.readFileSync(path.join(dir, manifest.schema), 'utf8') + return new Set([...sql.matchAll(CREATE_TABLE)].map((m) => m[1].toLowerCase())) +} + +function checkTableNames(id, tables) { + const allowed = LEGACY_TABLE_PREFIXES[id] || [] + const core = coreTableNames() + + for (const table of tables) { + if (core.has(table)) throw new Error(`schema fragment declares core table "${table}"`) + for (const other of modules.values()) { + if (other.tables.has(table)) { + throw new Error(`schema fragment declares "${table}", already owned by module "${other.id}"`) + } + } + const prefixed = table.startsWith(`${id}_`) || allowed.some((p) => table.startsWith(p)) + if (!prefixed) throw new Error(`schema fragment table "${table}" is not prefixed "${id}_"`) + } +} + +/** + * Does core already own this prefix in this tier? + * + * Asked of the LIVE tier router rather than a hardcoded list, so the check + * cannot drift the first time core adds a capability router — the spike's + * hardcoded table was already one prefix stale when it was written. Modules are + * loaded after every core mount, so the stack is complete by the time this runs, + * and `layer.match` is express's own matcher rather than a second-guess at its + * regexp grammar. + * + * Root-mounted layers are skipped: `use(noindex, requireAuth)` and the two + * `use('/', singletonRouter)` mounts match every path, and counting them would + * report every prefix as taken. + */ +function ownedByCore(tierRouter, prefix) { + return (tierRouter.stack || []).some( + (layer) => layer.regexp && !layer.regexp.fast_slash && layer.match(prefix), + ) +} + +function readManifest(dir, id, tierRouters) { + const file = path.join(dir, 'module.json') + const manifest = JSON.parse(fs.readFileSync(file, 'utf8')) + + for (const key of Object.keys(manifest)) { + // Rejected, not ignored: a typo'd key must be a loud failure rather than a + // silently inert setting the operator believes they configured. + if (!MANIFEST_KEYS.has(key)) throw new Error(`unknown key "${key}" in module.json`) + } + if (!ID.test(manifest.id || '')) throw new Error(`invalid id "${manifest.id}"`) + if (manifest.id !== id) throw new Error(`id "${manifest.id}" does not match directory "${id}"`) + if (!manifest.version) throw new Error('missing version') + if (!manifest.coreApi) throw new Error('missing coreApi') + if (!semver.satisfies(MODULE_API_VERSION, manifest.coreApi)) { + throw new Error(`needs core API ${manifest.coreApi}, this core is ${MODULE_API_VERSION}`) + } + + for (const [tier, prefixes] of Object.entries(manifest.mounts || {})) { + if (!TIERS.includes(tier)) throw new Error(`unknown tier "${tier}" in mounts`) + for (const prefix of prefixes) { + if (!PREFIX.test(prefix)) throw new Error(`bad prefix "${prefix}" in mounts.${tier}`) + if (ownedByCore(tierRouters[tier], prefix)) { + throw new Error(`prefix ${tier}${prefix} is owned by core`) + } + for (const other of modules.values()) { + if ((other.manifest.mounts?.[tier] || []).includes(prefix)) { + throw new Error(`prefix ${tier}${prefix} already registered by module "${other.id}"`) + } + } + } + } + + for (const slot of manifest.extensions || []) { + if (!CORE_SLOTS.has(slot)) throw new Error(`unknown extension slot "${slot}"`) + } + + if (manifest.schema && !manifest.purge) { + // A module that can create tables and cannot drop them leaves an operator + // with orphaned data and no supported way to remove it. + throw new Error('declares schema but no purge') + } + if (manifest.purge && !fs.existsSync(path.join(dir, manifest.purge))) { + throw new Error(`purge file "${manifest.purge}" is missing`) + } + return manifest +} + +// What the module registered must equal what it declared — in both directions. +function checkDeclared(record) { + const declared = record.manifest.mounts || {} + for (const tier of TIERS) { + const want = new Set(declared[tier] || []) + const got = new Set(record.routes[tier].keys()) + for (const p of got) if (!want.has(p)) throw new Error(`registered ${tier}${p} without declaring it`) + for (const p of want) if (!got.has(p)) throw new Error(`declared ${tier}${p} but never registered it`) + } +} + +// ── Load ─────────────────────────────────────────────────────────────────── + +/** + * Discover, validate, register and mount every module under MODULES_DIR. + * + * **Called exactly once, explicitly, from app.js**, after the three tier routers + * are required and before the app is exported. There is no lazy self-scan: the + * spike's was lazy and silent, so requiring the loader and reading the module + * list gave an empty array and no error (MODULE_API.md §7.6). Everything that + * reads the module list now throws until this has run. + * + * The ordering is not incidental. Core's mounts must already be on the tier + * routers, because that is what the prefix-collision check is asked about; and + * modules mount after them, so first-match-wins means a module could not shadow + * a core prefix even if the check were bypassed. + * + * Safe to call when the modules directory does not exist — that is the normal + * case for a bare core, and it is the state this PR ships in. + * + * @param {{public: Router, admin: Router, player: Router}} tierRouters + */ +function load(tierRouters) { + if (loaded) return + for (const tier of TIERS) { + if (!tierRouters || typeof tierRouters[tier] !== 'function') { + throw new Error(`modules.load: missing the "${tier}" tier router`) + } + } + loaded = true + + let entries = [] + try { + entries = fs.readdirSync(MODULES_DIR, { withFileTypes: true }) + .filter((e) => e.isDirectory()) + .map((e) => e.name) + .sort() // alphabetical: there is no dependency resolution, and any other + // order would imply a precedence nothing computes (§4.2) + } catch { + return // no modules directory is the normal case for a bare core + } + + for (const id of entries) { + const dir = path.join(MODULES_DIR, id) + if (!fs.existsSync(path.join(dir, 'module.json'))) continue + + const record = { + id, + dir, + manifest: null, + routes: { public: new Map(), admin: new Map(), player: new Map() }, + tables: new Set(), + called: new Set(), + state: 'installed', + reason: null, + } + + try { + record.manifest = readManifest(dir, id, tierRouters) + record.tables = tablesOf(dir, record.manifest) + checkTableNames(id, record.tables) + if (record.manifest.server) { + const entry = path.join(dir, record.manifest.server) + // eslint-disable-next-line global-require, import/no-dynamic-require + const register = require(entry) + if (typeof register !== 'function') throw new Error(`${record.manifest.server} does not export a function`) + register(buildCtx(id, dir), buildApi(record)) + checkDeclared(record) + } + record.state = 'registered' + modules.set(id, record) + log.info(`registered module "${id}" v${record.manifest.version}`, { + mounts: record.manifest.mounts, + }) + } catch (err) { + // A failure here is BEFORE any route was mounted, so this module's routes + // and nav are simply absent and the site comes up without it (§4.4). + record.state = 'startup_failed' + record.reason = err.message + record.manifest = record.manifest || { id, version: 'unknown' } + modules.set(id, record) + log.error(`module "${id}" failed to load — continuing without it`, { reason: err.message }) + } + } + + // Mounting is a SECOND pass, after every module has been validated, and not + // because it reads better. `ownedByCore` asks the live tier router what is + // already on it, so mounting inside the loop would make the first module's + // layers indistinguishable from core's — the second module claiming a taken + // prefix would be told it collided with core, naming the wrong culprit, and + // the module-versus-module check below it could never be reached. + for (const record of modules.values()) { + if (record.state === 'registered') mount(record, tierRouters) + } +} + +/** + * Mount one module's routers onto the tier routers, behind the dispatch guard. + * + * The guard is the other half of §4.4. A module that fails BEFORE this point has + * no routes at all; one that fails after — schema replay (PR 3), `onBoot` + * (PR 5) — keeps its URLs and answers 503, so `routes.manifest.json` never + * depends on whether a boot hook happened to succeed on the machine that + * generated it. `disabled` is 404 and unreachable until PR 5 wires + * `installed_modules` in; it is written here because the guard is the contract's + * §4.5, not a later addition. + */ +function mount(record, tierRouters) { + for (const tier of TIERS) { + for (const [prefix, router] of record.routes[tier]) { + tierRouters[tier].use(prefix, (req, res, next) => { + if (record.state === 'startup_failed') { + return res.status(503).json({ message: 'Module unavailable' }) + } + if (record.state === 'disabled') return res.status(404).json({ message: 'Not found' }) + return next() + }, router) + } + } +} + +// ── State ────────────────────────────────────────────────────────────────── + +// The states a loaded record may hold, deliberately a hardcoded subset rather +// than an import of model/modules/modules.model.js's STATES: that model reaches +// the database, and this file must stay require-able against a dead one. +// `installed` is not here because a record leaves load() resolved either way. +const RECORD_STATES = new Set(['registered', 'started', 'disabled', 'startup_failed']) + +/** + * Move a loaded module to a new state — the POST-mount transitions. + * + * Called by whoever ran the step that failed or the step that succeeded, because + * only they can know: PR 3's `ensureSchema()` replays the fragments, PR 5's boot + * dispatch runs `onBoot` and reconciles `installed_modules` (whose `disabled` + * rows are what first make the guard's 404 leg reachable). + * + * Unknown ids are ignored rather than thrown on: a module can be absent from the + * volume and still have a row, and a caller on the boot path must not turn that + * into everyone's failure. + */ +function setState(id, state, reason = null) { + if (!RECORD_STATES.has(state)) throw new Error(`unknown module state "${state}"`) + const record = modules.get(id) + if (!record) return + record.state = state + record.reason = reason +} + +// ── Introspection ────────────────────────────────────────────────────────── + +function assertLoaded(caller) { + if (!loaded) throw new Error(`modules.${caller}() before modules.load()`) +} + +/** + * Every module found on the volume, loaded or failed, in scan order. + * + * Throws rather than returning `[]` when load() has not run — the empty list is + * a real answer for a core with no modules installed, and a caller cannot tell + * the two apart (§7.6). + */ +function list() { + assertLoaded('list') + return [...modules.values()].map((r) => ({ + id: r.id, + name: r.manifest.name || r.id, + version: r.manifest.version, + state: r.state, + reason: r.reason, + capabilities: r.manifest.capabilities || [], + })) +} + +/** Absolute path of the modules directory. */ +const dir = () => MODULES_DIR + +module.exports = { load, list, setState, dir } diff --git a/server/src/modules/semver.js b/server/src/modules/semver.js new file mode 100644 index 0000000..062f8b5 --- /dev/null +++ b/server/src/modules/semver.js @@ -0,0 +1,47 @@ +// A deliberately tiny semver range check — enough for `coreApi` and no more. +// +// Supports `*`, an exact `x.y.z`, `^x.y.z` and `~x.y.z`. That is the whole +// grammar a module manifest is allowed to use (MODULE_API.md §1.1), so pulling +// in the `semver` package for it would add a dependency to the server for a +// twenty-line job. A range this parser does not understand is REJECTED rather +// than assumed to match — an unparseable range must not silently load a module +// against an API it was never tested on. + +const PARTS = /^(\d+)\.(\d+)\.(\d+)$/ + +function parse(version) { + const m = PARTS.exec(String(version).trim()) + if (!m) return null + return { major: Number(m[1]), minor: Number(m[2]), patch: Number(m[3]) } +} + +const gte = (a, b) => { + if (a.major !== b.major) return a.major > b.major + if (a.minor !== b.minor) return a.minor > b.minor + return a.patch >= b.patch +} + +/** + * Does `version` satisfy `range`? + * @param {string} version an exact x.y.z + * @param {string} range `*` | `x.y.z` | `^x.y.z` | `~x.y.z` + * @returns {boolean} false for anything unparseable, on either side + */ +function satisfies(version, range) { + const v = parse(version) + if (!v) return false + const raw = String(range).trim() + if (raw === '*') return true + + const op = raw[0] === '^' || raw[0] === '~' ? raw[0] : '' + const b = parse(op ? raw.slice(1) : raw) + if (!b) return false + + if (op === '') return v.major === b.major && v.minor === b.minor && v.patch === b.patch + if (!gte(v, b)) return false + // ^ allows minor+patch within the same major; ~ allows patch within the same minor. + if (op === '^') return v.major === b.major + return v.major === b.major && v.minor === b.minor +} + +module.exports = { satisfies, parse } diff --git a/server/src/modules/version.js b/server/src/modules/version.js new file mode 100644 index 0000000..8df4d79 --- /dev/null +++ b/server/src/modules/version.js @@ -0,0 +1,14 @@ +// The module API version — the single number a module's `coreApi` range is +// checked against (docs/website/MODULE_API.md §1.1). +// +// Bump minor when a member is ADDED to ctx or a new register* call appears; +// major when one is removed, its signature changes, or its behaviour changes +// without a signature change. A core-internal refactor behind an unchanged +// member is not a bump. +// +// Deliberately separate from PROTOCOL_VERSION (which versions the shard wire and +// has nothing to say about a website module) and from any module's own version. + +const MODULE_API_VERSION = '1.0.0' + +module.exports = { MODULE_API_VERSION } diff --git a/server/test/moduleLoader.test.js b/server/test/moduleLoader.test.js new file mode 100644 index 0000000..ccc40ae --- /dev/null +++ b/server/test/moduleLoader.test.js @@ -0,0 +1,444 @@ +// ── The loader's failure guarantees ──────────────────────────────────────── +// +// docs/website/MODULE_API.md §4.4 promises that a module which fails ANYWHERE in +// its lifecycle fails alone: the site comes up, other modules are unaffected, and +// the failure is recorded rather than thrown. That is the property most worth a +// test, because the failure paths are the ones nobody exercises by hand — every +// manual check runs the happy path. +// +// Each test builds a throwaway modules directory, points MODULES_DIR at it and +// re-requires the loader with a clean cache, so the scan is genuinely redone. +// MODULES_DIR is read into a const at require time (it has to be: the scan is +// synchronous and happens during app.js's require), so busting the cache is the +// only honest way to point the loader somewhere else. +// +// Point the pool at a closed port BEFORE requiring anything: buildCtx pulls in +// the models, which build a mariadb pool at require time. No query is ever run. +process.env.DB_HOST = '127.0.0.1' +process.env.DB_PORT = '59999' + +const fs = require('fs') +const os = require('os') +const path = require('path') + +const { test, beforeEach, after } = require('node:test') +const assert = require('node:assert/strict') + +const express = require('express') + +const db = require('../src/utils/db') +const { startApp } = require('./_helper') + +after(() => db.close()) + +let tmpRoot + +/** Three empty routers standing in for core's tiers — nothing owned, nothing gated. */ +const emptyTiers = () => ({ + public: express.Router(), + admin: express.Router(), + player: express.Router(), +}) + +function freshLoader(dir, tiers = emptyTiers()) { + process.env.MODULES_DIR = dir + delete require.cache[require.resolve('../src/modules/loader')] + // eslint-disable-next-line global-require + const loader = require('../src/modules/loader') + loader.load(tiers) + return loader +} + +function writeModule(id, { manifest = {}, server, schema } = {}) { + const dir = path.join(tmpRoot, id) + fs.mkdirSync(dir, { recursive: true }) + const full = { + id, + name: id, + version: '1.0.0', + coreApi: '^1.0.0', + ...(server === undefined ? {} : { server: 'index.js' }), + ...(schema === undefined ? {} : { schema: 'schema.sql', purge: 'purge.sql' }), + ...manifest, + } + fs.writeFileSync(path.join(dir, 'module.json'), JSON.stringify(full)) + if (server !== undefined) fs.writeFileSync(path.join(dir, 'index.js'), server) + if (schema !== undefined) { + fs.writeFileSync(path.join(dir, 'schema.sql'), schema) + fs.writeFileSync(path.join(dir, 'purge.sql'), '') + } + return dir +} + +/** A module that registers one router answering 200 at its prefix root. */ +const oneRoute = (prefix, tier = 'public') => ({ + manifest: { mounts: { [tier]: [prefix] } }, + server: `module.exports = (ctx, api) => { + const r = ctx.express.Router() + r.get('/', (req, res) => res.json({ ok: true })) + api.registerRoutes({ ${tier}: { '${prefix}': r } }) + }`, +}) + +const stateOf = (loader, id) => loader.list().find((m) => m.id === id) + +beforeEach(() => { + tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'rg-modules-')) +}) + +// ── Discovery and the explicit trigger ───────────────────────────────────── + +test('a missing modules directory is the normal case, not an error', () => { + const loader = freshLoader(path.join(tmpRoot, 'does-not-exist')) + assert.deepEqual(loader.list(), []) +}) + +test('reading the module list before load() throws instead of answering []', () => { + // §7.6. The spike's scan was lazy and silent, so a caller that required the + // loader and read nothing got an empty list — indistinguishable from a core + // with no modules installed. It cost one confusing failure; it now costs an + // error naming the missing call. + process.env.MODULES_DIR = tmpRoot + delete require.cache[require.resolve('../src/modules/loader')] + // eslint-disable-next-line global-require + const loader = require('../src/modules/loader') + assert.throws(() => loader.list(), /modules\.list\(\) before modules\.load\(\)/) +}) + +test('load() refuses to run without all three tier routers', () => { + writeModule('aaa', { server: 'module.exports = () => {}' }) + process.env.MODULES_DIR = tmpRoot + delete require.cache[require.resolve('../src/modules/loader')] + // eslint-disable-next-line global-require + const loader = require('../src/modules/loader') + // Not a module's failure — a wiring mistake in core, and the one thing in this + // file that is allowed to throw past the caller. + assert.throws(() => loader.load({ public: express.Router() }), /missing the "admin" tier router/) +}) + +test('load() is once-only, so a second call cannot double-mount', () => { + writeModule('aaa', oneRoute('/thing')) + const tiers = emptyTiers() + const loader = freshLoader(tmpRoot, tiers) + const before = tiers.public.stack.length + + loader.load(tiers) + assert.equal(tiers.public.stack.length, before) + assert.equal(loader.list().length, 1) +}) + +test('modules load in alphabetical order, since nothing computes a precedence', () => { + for (const id of ['ccc', 'aaa', 'bbb']) writeModule(id, { server: 'module.exports = () => {}' }) + const loader = freshLoader(tmpRoot) + assert.deepEqual(loader.list().map((m) => m.id), ['aaa', 'bbb', 'ccc']) +}) + +// ── The failing module fails alone ───────────────────────────────────────── + +test('a module whose entry point throws does not stop the others loading', () => { + writeModule('aaa', { server: 'module.exports = () => {}' }) + writeModule('bbb', { server: 'throw new Error("boom")' }) + writeModule('ccc', { server: 'module.exports = () => {}' }) + const loader = freshLoader(tmpRoot) + + assert.equal(stateOf(loader, 'aaa').state, 'registered') + assert.equal(stateOf(loader, 'ccc').state, 'registered') + + const bad = stateOf(loader, 'bbb') + assert.equal(bad.state, 'startup_failed') + assert.match(bad.reason, /boom/) +}) + +test('an entry point that exports something other than a function is rejected', () => { + writeModule('notfn', { server: 'module.exports = { register: () => {} }' }) + const loader = freshLoader(tmpRoot) + assert.match(stateOf(loader, 'notfn').reason, /does not export a function/) +}) + +test('a coreApi mismatch is refused before the module is required at all', () => { + // The entry point would throw if it ran; the version gate must run first. + writeModule('old', { + manifest: { coreApi: '^99.0.0' }, + server: 'throw new Error("should never be required")', + }) + const loader = freshLoader(tmpRoot) + const mod = stateOf(loader, 'old') + assert.equal(mod.state, 'startup_failed') + assert.match(mod.reason, /needs core API \^99\.0\.0/) +}) + +test('an unknown manifest key is rejected, not ignored', () => { + // A typo'd key must be loud: an operator who believes they configured + // something and silently did not is worse off than one who sees a failure. + writeModule('typo', { manifest: { mount: { public: ['/x'] } } }) + const loader = freshLoader(tmpRoot) + assert.match(stateOf(loader, 'typo').reason, /unknown key "mount"/) +}) + +test('a module id that does not match its directory is rejected', () => { + writeModule('onedir', { manifest: { id: 'another' } }) + const loader = freshLoader(tmpRoot) + // Recorded under the DIRECTORY name — the id it claimed is exactly what is + // not trusted here. + assert.match(stateOf(loader, 'onedir').reason, /does not match directory/) +}) + +test('an unknown extension slot is rejected; only core may declare a slot', () => { + writeModule('presumptuous', { manifest: { extensions: ['admin.users.detail', 'admin.invented'] } }) + const loader = freshLoader(tmpRoot) + assert.match(stateOf(loader, 'presumptuous').reason, /unknown extension slot "admin\.invented"/) +}) + +// ── Prefix ownership ─────────────────────────────────────────────────────── + +test('two modules cannot claim the same prefix; the first one wins', () => { + writeModule('aaa', oneRoute('/thing')) + writeModule('bbb', oneRoute('/thing')) + const loader = freshLoader(tmpRoot) + + assert.equal(stateOf(loader, 'aaa').state, 'registered') + assert.match(stateOf(loader, 'bbb').reason, /already registered by module "aaa"/) +}) + +test('a module cannot take a prefix core owns, asked of the live tier routers', () => { + // Deliberately the REAL routers rather than a hardcoded prefix list. The spike + // hardcoded core's ~24 prefixes and they were already stale; asking express + // itself is what stops the check drifting the next time core adds a capability + // router. + /* eslint-disable global-require */ + const real = { + public: require('../src/router/v1/public'), + admin: require('../src/router/v1/admin'), + player: require('../src/router/v1/player'), + } + /* eslint-enable global-require */ + + writeModule('greedy', { manifest: { mounts: { admin: ['/users'] } } }) + writeModule('alsogreedy', { manifest: { mounts: { public: ['/wiki'] } } }) + // Free in every tier core actually mounts, so it must be allowed through. + writeModule('polite', oneRoute('/widgets', 'player')) + + const loader = freshLoader(tmpRoot, real) + assert.match(stateOf(loader, 'greedy').reason, /owned by core/) + assert.match(stateOf(loader, 'alsogreedy').reason, /owned by core/) + assert.equal(stateOf(loader, 'polite').state, 'registered') +}) + +test('a root-mounted core layer does not make every prefix look taken', () => { + // public/index.js ends with `use('/', siteRouter)` and admin with the dashboard + // router; both match every path. Counting them would report every prefix as + // owned and no module could ever mount. + const tiers = emptyTiers() + tiers.public.use('/posts', express.Router()) + tiers.public.use('/', express.Router()) + + writeModule('fine', oneRoute('/widgets')) + writeModule('taken', oneRoute('/posts')) + const loader = freshLoader(tmpRoot, tiers) + + assert.equal(stateOf(loader, 'fine').state, 'registered') + assert.match(stateOf(loader, 'taken').reason, /owned by core/) +}) + +test('a prefix with a slash or a parameter in it is rejected', () => { + // A prefix that could contain either would let a module reach outside the slot + // it was given. + writeModule('nested', { manifest: { mounts: { public: ['/a/b'] } } }) + writeModule('parameterised', { manifest: { mounts: { public: ['/:id'] } } }) + const loader = freshLoader(tmpRoot) + assert.match(stateOf(loader, 'nested').reason, /bad prefix "\/a\/b"/) + assert.match(stateOf(loader, 'parameterised').reason, /bad prefix "\/:id"/) +}) + +test('registering a prefix that was never declared is rejected', () => { + // module.json is what the admin panel, the collision check and the reviewer + // all read, so it has to be the truth rather than a hint. + writeModule('sneaky', { + manifest: { mounts: { public: ['/declared'] } }, + server: `module.exports = (ctx, api) => api.registerRoutes({ + public: { '/declared': ctx.express.Router(), '/undeclared': ctx.express.Router() }, + })`, + }) + const loader = freshLoader(tmpRoot) + assert.match(stateOf(loader, 'sneaky').reason, /registered public\/undeclared without declaring it/) +}) + +test('declaring a prefix and never registering it is rejected too', () => { + writeModule('forgetful', { + manifest: { mounts: { public: ['/a', '/b'] } }, + server: "module.exports = (ctx, api) => api.registerRoutes({ public: { '/a': ctx.express.Router() } })", + }) + const loader = freshLoader(tmpRoot) + assert.match(stateOf(loader, 'forgetful').reason, /declared public\/b but never registered it/) +}) + +test('registering the same thing twice is an error, not a silent overwrite', () => { + writeModule('twice', { + manifest: { mounts: { public: ['/x'] } }, + server: `module.exports = (ctx, api) => { + api.registerRoutes({ public: { '/x': ctx.express.Router() } }) + api.registerRoutes({ public: { '/x': ctx.express.Router() } }) + }`, + }) + const loader = freshLoader(tmpRoot) + assert.match(stateOf(loader, 'twice').reason, /registerRoutes\(\) called twice/) +}) + +// ── Schema fragment validation (the replay itself is PR 3) ───────────────── + +test('a schema fragment declaring a core table is rejected', () => { + writeModule('thief', { schema: 'CREATE TABLE IF NOT EXISTS users (id INT);' }) + const loader = freshLoader(tmpRoot) + assert.match(stateOf(loader, 'thief').reason, /declares core table "users"/) +}) + +test('a schema fragment table must carry the module id as a prefix', () => { + writeModule('mine', { schema: 'CREATE TABLE IF NOT EXISTS widgets (id INT);' }) + assert.match(stateOf(freshLoader(tmpRoot), 'mine').reason, /not prefixed "mine_"/) + + tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'rg-modules-')) + writeModule('mine', { schema: 'CREATE TABLE IF NOT EXISTS mine_widgets (id INT);' }) + assert.equal(stateOf(freshLoader(tmpRoot), 'mine').state, 'registered') +}) + +test('two modules cannot own the same table either', () => { + writeModule('aaa', { schema: 'CREATE TABLE IF NOT EXISTS aaa_shared (id INT);' }) + writeModule('bbb', { + manifest: { id: 'bbb' }, + schema: 'CREATE TABLE IF NOT EXISTS aaa_shared (id INT);', + }) + const loader = freshLoader(tmpRoot) + assert.equal(stateOf(loader, 'aaa').state, 'registered') + assert.match(stateOf(loader, 'bbb').reason, /already owned by module "aaa"/) +}) + +test('declaring a schema without a purge is rejected', () => { + const dir = path.join(tmpRoot, 'noway') + fs.mkdirSync(dir, { recursive: true }) + fs.writeFileSync( + path.join(dir, 'module.json'), + JSON.stringify({ id: 'noway', name: 'x', version: '1.0.0', coreApi: '^1.0.0', schema: 'schema.sql' }), + ) + const loader = freshLoader(tmpRoot) + // A module that can create tables and cannot drop them leaves an operator with + // orphaned data and no supported way to remove it. + assert.match(stateOf(loader, 'noway').reason, /declares schema but no purge/) +}) + +test('a declared purge file that is not there is rejected', () => { + writeModule('gone', { schema: 'CREATE TABLE IF NOT EXISTS gone_x (id INT);' }) + fs.unlinkSync(path.join(tmpRoot, 'gone', 'purge.sql')) + const loader = freshLoader(tmpRoot) + assert.match(stateOf(loader, 'gone').reason, /purge file "purge\.sql" is missing/) +}) + +// ── Mounting and the dispatch guard ──────────────────────────────────────── + +test('a registered module answers on its prefix; a failed one is simply absent', async () => { + writeModule('good', oneRoute('/widgets')) + writeModule('bad', { ...oneRoute('/broken'), server: 'throw new Error("boom")' }) + + const tiers = emptyTiers() + freshLoader(tmpRoot, tiers) + const app = await startApp((a) => a.use('/public', tiers.public)) + + try { + assert.equal((await fetch(`${app.url}/public/widgets`)).status, 200) + // Failed BEFORE mounting, so its routes are not absent-with-a-503 — they do + // not exist at all (§4.4, left-hand column). + assert.equal((await fetch(`${app.url}/public/broken`)).status, 404) + } finally { + await app.close() + } +}) + +test('a module that fails AFTER mounting keeps its URLs and answers 503', async () => { + // The right-hand column of §4.4, and the reason routes.manifest.json can be + // generated off a dead database: the URL surface must not depend on whether a + // boot step succeeded on the generating machine. PR 3 (schema replay) and PR 5 + // (onBoot) are the two things that will trip this in real life; here the state + // is moved by hand, because the loader is the thing under test. + writeModule('later', oneRoute('/widgets')) + + const tiers = emptyTiers() + const loader = freshLoader(tmpRoot, tiers) + const app = await startApp((a) => a.use('/public', tiers.public)) + + try { + assert.equal((await fetch(`${app.url}/public/widgets`)).status, 200) + + assert.equal(stateOf(loader, 'later').state, 'registered') + + loader.setState('later', 'startup_failed', 'schema fragment blew up') + assert.equal((await fetch(`${app.url}/public/widgets`)).status, 503) + assert.equal(stateOf(loader, 'later').reason, 'schema fragment blew up') + + // The 404 leg becomes reachable for real in PR 5, when the boot reconcile + // reads a `disabled` row out of installed_modules. A disabled module is + // mounted and guarded, never unmounted (§4.5) — same reason as the 503. + loader.setState('later', 'disabled') + assert.equal((await fetch(`${app.url}/public/widgets`)).status, 404) + + loader.setState('later', 'started') + assert.equal((await fetch(`${app.url}/public/widgets`)).status, 200) + } finally { + await app.close() + } +}) + +test('setState refuses a state that is not one, and shrugs at an unknown id', () => { + writeModule('here', oneRoute('/widgets')) + const loader = freshLoader(tmpRoot) + + assert.throws(() => loader.setState('here', 'enabled'), /unknown module state "enabled"/) + // An id with no record is not the boot path's problem to escalate. + assert.doesNotThrow(() => loader.setState('never-installed', 'started')) +}) + +// ── ctx ──────────────────────────────────────────────────────────────────── + +test('ctx exposes exactly the documented surface, and is frozen', () => { + const seen = path.join(tmpRoot, 'probe-out.json') + writeModule('probe', { + server: `const fs = require('fs') + module.exports = (ctx) => { + let mutable = true + try { ctx.db.query = null; mutable = ctx.db.query === null } catch { mutable = false } + fs.writeFileSync(${JSON.stringify(seen)}, JSON.stringify({ + keys: Object.keys(ctx).sort(), + middleware: Object.keys(ctx.middleware).sort(), + moduleId: ctx.moduleId, + mutable, + })) + }`, + }) + assert.equal(stateOf(freshLoader(tmpRoot), 'probe').state, 'registered') + + const probe = JSON.parse(fs.readFileSync(seen, 'utf8')) + assert.deepEqual(probe.keys, [ + 'auth', 'db', 'express', 'log', 'middleware', 'moduleId', 'paths', + 'posts', 'push', 'secretBox', 'settings', 'uploads', 'validator', + ]) + assert.deepEqual(probe.middleware, ['noindex', 'requireAuth', 'requireRole', 'siteMode', 'validate']) + assert.equal(probe.moduleId, 'probe') + assert.equal(probe.mutable, false, 'ctx members must be frozen') +}) + +test('the register calls PR 4 and PR 5 own throw rather than silently accepting', () => { + // An accepting no-op would let a module believe it had registered a + // notification stream or a boot hook and fail silently at the far end. + for (const [call, pr] of [ + ['registerExtension', 4], + ['registerNotificationStreams', 4], + ['registerAnnounceLeg', 4], + ['onBoot', 5], + ['onShutdown', 5], + ]) { + tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'rg-modules-')) + writeModule('early', { server: `module.exports = (ctx, api) => api.${call}(() => {})` }) + assert.match( + stateOf(freshLoader(tmpRoot), 'early').reason, + new RegExp(`${call}: not available until phase 2 PR ${pr}`), + ) + } +}) -- 2.49.1 From 2892d01b2406ec58f774198248b48a71dfa7975c Mon Sep 17 00:00:00 2001 From: wtclaude Date: Mon, 10 Aug 2026 16:56:58 -0500 Subject: [PATCH 03/30] feat(modules): replay module schema fragments after core's Phase 2, PR 3 of docs/website/MODULE_SYSTEM.md 2.7. ensureSchema() now replays every installed module's schema fragment immediately after core's schema.sql, per MODULE_API.md 2.6. The work splits across two files on the line of whether a database is needed to know the answer. loader.js VALIDATES a fragment at load time, before anything is mounted, because every rule 2.6 states about the SQL is knowable by reading it; a module that breaks one never mounts (4.4, left column). modules/schema.js EXECUTES it, so the only failures there are the ones the database alone could report, and those are post-mount and answer 503 (4.4, right column). Validation is a leading-verb allowlist -- CREATE, ALTER, INSERT, UPDATE, the four core's own schema.sql uses -- rather than the DROP denylist 2.6 words it as. A fragment is replayed on every boot, so TRUNCATE and DELETE would empty a table at each restart and RENAME would fail at the second one; a denylist only ever bans what somebody thought of. A CREATE TABLE missing IF NOT EXISTS is rejected for the same reason: it works once and fails every boot after, which presents to an operator as a module that broke on restart. The splitter moves to utils/sqlStatements.js so core's schema and a fragment are split by literally the same code, which is what 2.6 promises. It is its own file rather than an export of utils/db.js because the loader validates fragments at require time and must not drag the mariadb pool into app.js's require chain. The replay sits outside ensureSchema's wait-for-the-database retry loop: a fragment that throws is one module's failure, not a signal the database is coming up, and retrying core's whole schema nine more times over one module's bad SQL would turn a 503'd module into a two-minute boot. Found while wiring it: db/seed.js calls ensureSchema() standalone for `npm run seed`, without ever requiring app.js, so the loader has not scanned and fragments()'s 7.6 throw would have broken seeding outright. The replay asks isLoaded() and logs the skip rather than swallowing it -- a booting server quietly getting no module tables is the thing 7.6 exists to prevent. Verification: - 856 server tests pass, 14 new. moduleSchema.test.js injects the query fn, so the exact statements and their order are asserted with the pool at a dead port like every other suite. - routes.manifest.json and routes.guards.json diffs are zero lines, 229 routes -- the phase 2 exit criterion. swagger-output.json regenerates byte-identical. - Run for real against the local MariaDB with two fixture modules: a good fragment created its table, applied its ALTER and seeded its row; a fragment whose SQL passes validation but the server rejects (`id NOTATYPE`) marked only that module startup_failed, its route answering 503 while the other answered 200; a second ensureSchema on the same database was a clean no-op. Co-Authored-By: Claude --- server/src/modules/loader.js | 91 ++++++++++- server/src/modules/schema.js | 84 ++++++++++ server/src/utils/db.js | 34 ++-- server/src/utils/sqlStatements.js | 37 +++++ server/test/moduleLoader.test.js | 71 ++++++++- server/test/moduleSchema.test.js | 247 ++++++++++++++++++++++++++++++ 6 files changed, 541 insertions(+), 23 deletions(-) create mode 100644 server/src/modules/schema.js create mode 100644 server/src/utils/sqlStatements.js create mode 100644 server/test/moduleSchema.test.js diff --git a/server/src/modules/loader.js b/server/src/modules/loader.js index 0fc7e6f..6b1d958 100644 --- a/server/src/modules/loader.js +++ b/server/src/modules/loader.js @@ -18,16 +18,22 @@ // boot (§4.4). // // What is deliberately NOT here yet, each landing with the PR that first calls -// it (§2.7): schema-fragment replay (PR 3), the three de-entanglement registries -// (PR 4), boot/shutdown hook dispatch and the `installed_modules` reconcile -// (PR 5), GET /api/v1/public/modules and the client chunk's static mount -// (PRs 6-7). Until PR 5 the state a module carries is in memory only. +// it (§2.7): the three de-entanglement registries (PR 4), boot/shutdown hook +// dispatch and the `installed_modules` reconcile (PR 5), GET +// /api/v1/public/modules and the client chunk's static mount (PRs 6-7). Until +// PR 5 the state a module carries is in memory only. +// +// PR 3 added the fragment half of the schema story: this file VALIDATES a +// fragment (statement by statement, at load time, before anything is mounted) +// and publishes it through `fragments()`. Replaying it needs a database, so it +// belongs to modules/schema.js, which utils/db.js calls after core's schema. const fs = require('fs') const path = require('path') const { MODULE_API_VERSION } = require('./version') const semver = require('./semver') +const { splitStatements } = require('../utils/sqlStatements') const log = require('../utils/logger')('modules') @@ -193,10 +199,52 @@ function coreTableNames() { return coreTables } -/** Every table name a schema fragment declares. Throws if the file is unreadable. */ +// The only leading verbs a fragment may use — an allowlist, not a DROP denylist. +// +// §2.6 bans `DROP`, but a denylist only ever bans what somebody thought of, and +// core's own schema.sql needs exactly four verbs: CREATE, ALTER, INSERT, UPDATE. +// Anything else in a file that is REPLAYED ON EVERY BOOT is a mistake worth +// failing on — TRUNCATE and DELETE would empty a table every restart, RENAME +// would break on the second one, and GRANT/SET/USE are core's business, not a +// module's. CREATE covers CREATE INDEX as well as CREATE TABLE. +// +// This is a leading-verb check and says so: `ALTER TABLE x DROP COLUMN y` passes +// it. Catching that needs a SQL parser, which is a large dependency to take on +// for a rule whose real job is stopping the obvious foot-gun early. +const ALLOWED_VERBS = new Set(['CREATE', 'ALTER', 'INSERT', 'UPDATE']) + +const CREATE_TABLE_ANY = /^CREATE\s+TABLE\s+(?:IF\s+NOT\s+EXISTS\s+)?/i +const CREATE_TABLE_GUARDED = /^CREATE\s+TABLE\s+IF\s+NOT\s+EXISTS\s+/i + +/** + * Read and validate a module's schema fragment; return every table it declares. + * + * Validation happens HERE, at load time, and not in modules/schema.js where the + * fragment is replayed, because every rule §2.6 states is knowable by reading + * the file — no database required. Failing at load means a module with a bad + * fragment never mounts at all (§4.4's first column: routes and nav simply + * absent), rather than mounting, 503ing, and leaving whatever its fragment did + * manage to execute behind it. + * + * Throws if the file is unreadable or breaks a rule. + */ function tablesOf(dir, manifest) { if (!manifest.schema) return new Set() - const sql = fs.readFileSync(path.join(dir, manifest.schema), 'utf8') + const file = path.join(dir, manifest.schema) + const sql = fs.readFileSync(file, 'utf8') + + for (const statement of splitStatements(sql)) { + const verb = (statement.match(/^\w+/) || [''])[0].toUpperCase() + if (!ALLOWED_VERBS.has(verb)) { + throw new Error(`schema fragment statement starts with "${verb}" (allowed: ${[...ALLOWED_VERBS].join(', ')})`) + } + // A bare CREATE TABLE succeeds exactly once and fails every boot after it, + // which presents as a module that worked until the first restart. + if (CREATE_TABLE_ANY.test(statement) && !CREATE_TABLE_GUARDED.test(statement)) { + throw new Error('schema fragment has a CREATE TABLE without IF NOT EXISTS') + } + } + return new Set([...sql.matchAll(CREATE_TABLE)].map((m) => m[1].toLowerCase())) } @@ -448,6 +496,18 @@ function assertLoaded(caller) { if (!loaded) throw new Error(`modules.${caller}() before modules.load()`) } +/** + * Has load() run in this process? + * + * The one legitimate reason to ask instead of just calling an accessor: a + * process that never required app.js and so has no module list to be wrong + * about. `npm run seed` (db/seed.js) is exactly that — it calls ensureSchema() + * standalone, and the fragment replay has to be able to tell "this is the seed + * script" from "the server booted and something is mis-ordered", which is the + * distinction §7.6's throw exists to preserve everywhere else. + */ +const isLoaded = () => loaded + /** * Every module found on the volume, loaded or failed, in scan order. * @@ -467,7 +527,24 @@ function list() { })) } +/** + * Every schema fragment waiting to be replayed, in scan order. + * + * `registered` only: a module that failed validation must not get its tables + * created (it is not going to run), and one already `started` has had them. The + * absolute path is resolved here rather than handed out as a manifest-relative + * name, so the replay never has to know how a module directory is laid out. + * + * @returns {{id: string, file: string}[]} + */ +function fragments() { + assertLoaded('fragments') + return [...modules.values()] + .filter((r) => r.state === 'registered' && r.manifest.schema) + .map((r) => ({ id: r.id, file: path.join(r.dir, r.manifest.schema) })) +} + /** Absolute path of the modules directory. */ const dir = () => MODULES_DIR -module.exports = { load, list, setState, dir } +module.exports = { load, list, setState, fragments, isLoaded, dir } diff --git a/server/src/modules/schema.js b/server/src/modules/schema.js new file mode 100644 index 0000000..13b59d0 --- /dev/null +++ b/server/src/modules/schema.js @@ -0,0 +1,84 @@ +// ── Module schema fragment replay ────────────────────────────────────────── +// +// Phase 2, PR 3 of docs/website/MODULE_SYSTEM.md §2.7. Normative contract: +// docs/website/MODULE_API.md §2.6 (fragments) and §4.4 (failure is a state). +// +// utils/db.js calls replayFragments() once, immediately after core's schema.sql +// is in place and before seedDefaults(), so that by the time a module's onBoot +// runs (PR 5) its tables exist. +// +// The split of responsibility with loader.js is worth stating, because it is the +// reason there are two files: +// +// loader.js VALIDATES a fragment — at load time, with no database, before +// anything is mounted. Every rule §2.6 states about the SQL is +// knowable by reading it, so a fragment that breaks one costs the +// module its mount entirely (§4.4, first column). +// schema.js EXECUTES it. Only reachable failures live here: the database +// rejecting a statement it could not have known was bad. Those are +// post-mount, so they 503 (§4.4, second column). +// +// The property this file exists to keep: **a fragment that fails takes down its +// own module and nothing else.** Not core's boot, not another module's tables. + +const fs = require('fs') + +const { splitStatements } = require('../utils/sqlStatements') + +const log = require('../utils/logger')('modules') + +/** + * Replay every installed module's schema fragment, in scan order. + * + * Never throws. A module whose fragment fails is moved to `startup_failed` with + * the database's own message as the reason, its routes answer 503 through the + * dispatch guard the loader already mounted, and the next module is replayed as + * if nothing happened. + * + * Partial application is accepted rather than compensated for: MariaDB commits + * each DDL statement implicitly, so a fragment failing at statement three has + * already created the first two tables and no wrapping transaction could undo + * them. Since every statement is required to be idempotent (§2.6), the fix is + * for the operator to correct the fragment and reboot — the surviving tables are + * re-CREATE-IF-NOT-EXISTSed harmlessly and the replay carries on past them. + * + * @param {object} [deps] injection seam for tests — the whole point of this + * function taking arguments at all, since the server suite runs with the pool + * pointed at a dead port. + * @param {(sql: string) => Promise} [deps.query] + * @param {object} [deps.modules] the loader + */ +async function replayFragments({ query, modules } = {}) { + /* eslint-disable global-require */ + const run = query || require('../utils/db').query + const loader = modules || require('./loader') + /* eslint-enable global-require */ + + // Not an error: `npm run seed` calls ensureSchema() without ever requiring + // app.js, so no scan has happened and there is genuinely nothing to replay. + // Logged rather than silently skipped — the one thing that must not happen is + // a booting server quietly getting no module tables (§7.6). + if (!loader.isLoaded()) { + log.info('no module scan in this process — skipping schema fragment replay') + return + } + + for (const { id, file } of loader.fragments()) { + try { + const statements = splitStatements(fs.readFileSync(file, 'utf8')) + for (const statement of statements) { + // Serially, and awaited: a fragment's ALTER TABLE routinely depends on + // the CREATE TABLE above it. + await run(statement) + } + log.info(`schema ensured for module "${id}"`, { statements: statements.length }) + } catch (err) { + loader.setState(id, 'startup_failed', err.message) + log.error(`module "${id}" schema fragment failed — its routes will answer 503`, { + reason: err.message, + }) + } + } +} + +module.exports = { replayFragments } diff --git a/server/src/utils/db.js b/server/src/utils/db.js index 362d41d..713784b 100644 --- a/server/src/utils/db.js +++ b/server/src/utils/db.js @@ -4,6 +4,7 @@ const mariadb = require('mariadb') require('dotenv').config() const log = require('./logger')('db') +const { splitStatements } = require('./sqlStatements') const pool = mariadb.createPool({ host: process.env.DB_HOST || '127.0.0.1', @@ -44,28 +45,31 @@ const SCHEMA_PATH = path.join(__dirname, '..', '..', 'db', 'schema.sql') /** * Create tables if they do not exist. Idempotent. Retries while the DB is still * coming up (important under docker-compose even with a healthcheck). + * + * Once core's schema is in place, every installed module's schema fragment is + * replayed after it (MODULE_API.md §2.6). That step is deliberately OUTSIDE the + * retry loop: a fragment that throws is that module's failure, not a signal the + * database is still coming up, and retrying core's whole schema nine more times + * because one module shipped bad SQL would turn a 503'd module into a two-minute + * boot. It is also why this file knows nothing about modules beyond the one call + * below — the discovery, splitting and per-module failure handling all live in + * modules/schema.js, required lazily so that requiring the pool never drags the + * loader in with it. */ async function ensureSchema({ retries = 10, delayMs = 2000 } = {}) { + await ensureCoreSchema({ retries, delayMs }) + // eslint-disable-next-line global-require + await require('../modules/schema').replayFragments() +} + +/** Core's own schema.sql, with the wait-for-the-database retry. */ +async function ensureCoreSchema({ retries, delayMs }) { for (let attempt = 1; attempt <= retries; attempt++) { try { const conn = await pool.getConnection() try { const sql = fs.readFileSync(SCHEMA_PATH, 'utf8') - // Strip `--` comments (full-line AND trailing) before splitting — so a - // leading comment block doesn't get glued onto the statement that follows - // it, and a `;` inside a trailing comment can't chop a statement in half. - // Safe because the schema never puts `--` inside a string literal. - const statements = sql - .split('\n') - .map((line) => { - const i = line.indexOf('--') - return i === -1 ? line : line.slice(0, i) - }) - .join('\n') - .split(';') - .map((s) => s.trim()) - .filter((s) => s.length > 0) - for (const statement of statements) { + for (const statement of splitStatements(sql)) { await conn.query(statement) } log.info('schema ensured') diff --git a/server/src/utils/sqlStatements.js b/server/src/utils/sqlStatements.js new file mode 100644 index 0000000..9c86f6c --- /dev/null +++ b/server/src/utils/sqlStatements.js @@ -0,0 +1,37 @@ +// ── Splitting a .sql file into statements ────────────────────────────────── +// +// Extracted from utils/db.js so that core's schema.sql and a module's schema +// fragment are split by literally the same code. MODULE_API.md §2.6 promises a +// fragment is replayed "statement by statement, split the same way" — with two +// copies of this that promise would hold only until one of them was edited. +// +// It lives in its own file rather than being exported from utils/db.js because +// modules/loader.js validates fragments at require time and must not pull the +// mariadb pool into app.js's require chain to do it. + +/** + * Split a .sql file into individual statements. + * + * Strips `--` comments (full-line AND trailing) before splitting — so a leading + * comment block doesn't get glued onto the statement that follows it, and a `;` + * inside a trailing comment can't chop a statement in half. Safe because neither + * core's schema nor a conforming fragment puts `--` inside a string literal + * (§2.6 states that as a rule a fragment must follow). + * + * @param {string} sql + * @returns {string[]} non-empty, trimmed statements in file order + */ +function splitStatements(sql) { + return sql + .split('\n') + .map((line) => { + const i = line.indexOf('--') + return i === -1 ? line : line.slice(0, i) + }) + .join('\n') + .split(';') + .map((s) => s.trim()) + .filter((s) => s.length > 0) +} + +module.exports = { splitStatements } diff --git a/server/test/moduleLoader.test.js b/server/test/moduleLoader.test.js index ccc40ae..e6212f0 100644 --- a/server/test/moduleLoader.test.js +++ b/server/test/moduleLoader.test.js @@ -284,7 +284,7 @@ test('registering the same thing twice is an error, not a silent overwrite', () assert.match(stateOf(loader, 'twice').reason, /registerRoutes\(\) called twice/) }) -// ── Schema fragment validation (the replay itself is PR 3) ───────────────── +// ── Schema fragment validation (the replay itself is moduleSchema.test.js) ── test('a schema fragment declaring a core table is rejected', () => { writeModule('thief', { schema: 'CREATE TABLE IF NOT EXISTS users (id INT);' }) @@ -332,6 +332,75 @@ test('a declared purge file that is not there is rejected', () => { assert.match(stateOf(loader, 'gone').reason, /purge file "purge\.sql" is missing/) }) +test('a fragment may only use the four verbs core\'s own schema.sql uses', () => { + // An allowlist rather than a DROP denylist. §2.6 bans DROP, but this file is + // REPLAYED ON EVERY BOOT, so TRUNCATE and DELETE would empty a table at every + // restart and RENAME would fail at the second one — a denylist only ever bans + // what somebody thought of. + for (const [id, sql, verb] of [ + ['dropper', 'DROP TABLE dropper_x;', 'DROP'], + ['nuker', 'TRUNCATE TABLE nuker_x;', 'TRUNCATE'], + ['wiper', 'DELETE FROM wiper_x;', 'DELETE'], + ['granter', 'GRANT ALL ON *.* TO app;', 'GRANT'], + ]) { + writeModule(id, { schema: sql }) + const reason = stateOf(freshLoader(tmpRoot), id).reason + assert.match(reason, new RegExp(`starts with "${verb}"`)) + } +}) + +test('a fragment may INSERT and UPDATE its own seed data', () => { + // Core's schema.sql does both (INSERT IGNORE INTO settings, one UPDATE), so a + // module that seeds a lookup table the same way must not be rejected. + writeModule('seeder', { + schema: [ + 'CREATE TABLE IF NOT EXISTS seeder_kinds (id INT PRIMARY KEY, label VARCHAR(32));', + "INSERT IGNORE INTO seeder_kinds (id, label) VALUES (1, 'first');", + "UPDATE seeder_kinds SET label = 'first' WHERE id = 1;", + ].join('\n'), + }) + assert.equal(stateOf(freshLoader(tmpRoot), 'seeder').state, 'registered') +}) + +test('a CREATE TABLE without IF NOT EXISTS is rejected', () => { + // It succeeds exactly once and fails every boot after it, which presents as a + // module that worked until the first restart — the worst kind of bug to ship + // to an operator, and free to catch by reading the file. + writeModule('once', { schema: 'CREATE TABLE once_x (id INT);' }) + assert.match(stateOf(freshLoader(tmpRoot), 'once').reason, /CREATE TABLE without IF NOT EXISTS/) +}) + +test('a fragment carrying an unreadable file fails the module, not the boot', () => { + writeModule('missing', { schema: 'CREATE TABLE IF NOT EXISTS missing_x (id INT);' }) + fs.unlinkSync(path.join(tmpRoot, 'missing', 'schema.sql')) + writeModule('fine', oneRoute('/ok')) + + const loader = freshLoader(tmpRoot) + assert.equal(stateOf(loader, 'missing').state, 'startup_failed') + assert.equal(stateOf(loader, 'fine').state, 'registered') +}) + +test('fragments() lists only registered modules that have one', () => { + writeModule('withdb', { schema: 'CREATE TABLE IF NOT EXISTS withdb_x (id INT);' }) + writeModule('nodb', oneRoute('/plain')) + writeModule('broken', { schema: 'CREATE TABLE IF NOT EXISTS not_mine (id INT);' }) + + const loader = freshLoader(tmpRoot) + const frags = loader.fragments() + + assert.deepEqual(frags.map((f) => f.id), ['withdb']) + // An absolute path, so the replay never has to know how a module dir is laid out. + assert.equal(frags[0].file, path.join(tmpRoot, 'withdb', 'schema.sql')) +}) + +test('fragments() before load() throws, like every other accessor', () => { + process.env.MODULES_DIR = tmpRoot + delete require.cache[require.resolve('../src/modules/loader')] + // eslint-disable-next-line global-require + const loader = require('../src/modules/loader') + assert.throws(() => loader.fragments(), /modules\.fragments\(\) before modules\.load\(\)/) +}) + // ── Mounting and the dispatch guard ──────────────────────────────────────── test('a registered module answers on its prefix; a failed one is simply absent', async () => { diff --git a/server/test/moduleSchema.test.js b/server/test/moduleSchema.test.js new file mode 100644 index 0000000..66e714f --- /dev/null +++ b/server/test/moduleSchema.test.js @@ -0,0 +1,247 @@ +// ── Replaying module schema fragments ────────────────────────────────────── +// +// Phase 2, PR 3. The contract is MODULE_API.md §2.6 (a fragment is replayed by +// the same ensureSchema() that replays core's, statement by statement, split the +// same way) and §4.4 (a failure after mounting is a state, not a crash). +// +// The property under test throughout, as in moduleLoader.test.js: **the failing +// module fails alone.** A fragment that blows up must cost its own module its +// routes and nothing else — not core's boot, not the next module's tables. +// +// No database is involved. `replayFragments` takes its `query` as an injectable +// dependency precisely so this suite can assert on the exact statements that +// would have been executed, in order, with the pool pointed at a dead port like +// every other suite here. +process.env.DB_HOST = '127.0.0.1' +process.env.DB_PORT = '59999' + +const fs = require('fs') +const os = require('os') +const path = require('path') + +const { test, beforeEach, after } = require('node:test') +const assert = require('node:assert/strict') + +const express = require('express') + +const db = require('../src/utils/db') +const { replayFragments } = require('../src/modules/schema') +const { splitStatements } = require('../src/utils/sqlStatements') +const { startApp } = require('./_helper') + +after(() => db.close()) + +let tmpRoot + +const emptyTiers = () => ({ + public: express.Router(), + admin: express.Router(), + player: express.Router(), +}) + +function freshLoader(dir, tiers = emptyTiers()) { + process.env.MODULES_DIR = dir + delete require.cache[require.resolve('../src/modules/loader')] + // eslint-disable-next-line global-require + const loader = require('../src/modules/loader') + loader.load(tiers) + return loader +} + +/** A module with a valid fragment, and optionally a route to watch 503 later. */ +function writeModule(id, { schema, mounts } = {}) { + const dir = path.join(tmpRoot, id) + fs.mkdirSync(dir, { recursive: true }) + fs.writeFileSync(path.join(dir, 'module.json'), JSON.stringify({ + id, + name: id, + version: '1.0.0', + coreApi: '^1.0.0', + ...(schema === undefined ? {} : { schema: 'schema.sql', purge: 'purge.sql' }), + ...(mounts === undefined ? {} : { mounts, server: 'index.js' }), + })) + if (schema !== undefined) { + fs.writeFileSync(path.join(dir, 'schema.sql'), schema) + fs.writeFileSync(path.join(dir, 'purge.sql'), `DROP TABLE IF EXISTS ${id}_x;`) + } + if (mounts !== undefined) { + const [prefix] = mounts.public + fs.writeFileSync(path.join(dir, 'index.js'), `module.exports = (ctx, api) => { + const r = ctx.express.Router() + r.get('/', (req, res) => res.json({ ok: true })) + api.registerRoutes({ public: { '${prefix}': r } }) + }`) + } + return dir +} + +/** A query fn that records what it was asked to run, and can be told to fail. */ +function recorder(failOn = null) { + const ran = [] + return { + ran, + query: async (sql) => { + ran.push(sql) + if (failOn && sql.includes(failOn)) throw new Error(`ER_PARSE_ERROR: near "${failOn}"`) + return [] + }, + } +} + +const stateOf = (loader, id) => loader.list().find((m) => m.id === id) + +beforeEach(() => { + tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'rg-schema-')) +}) + +// ── The happy path ───────────────────────────────────────────────────────── + +test('a fragment is replayed statement by statement, in file order', async () => { + writeModule('alpha', { + schema: [ + 'CREATE TABLE IF NOT EXISTS alpha_a (id INT);', + 'CREATE TABLE IF NOT EXISTS alpha_b (id INT);', + 'ALTER TABLE alpha_a ADD COLUMN IF NOT EXISTS name VARCHAR(64);', + ].join('\n'), + }) + const loader = freshLoader(tmpRoot) + const rec = recorder() + + await replayFragments({ query: rec.query, modules: loader }) + + assert.equal(rec.ran.length, 3) + // Order is load-bearing, not incidental: the ALTER depends on the CREATE above + // it, which is why the replay awaits each statement rather than Promise.all. + assert.match(rec.ran[0], /alpha_a/) + assert.match(rec.ran[1], /alpha_b/) + assert.match(rec.ran[2], /^ALTER TABLE alpha_a/) + assert.equal(stateOf(loader, 'alpha').state, 'registered') +}) + +test('a module with no fragment is skipped, not replayed as empty', async () => { + writeModule('nodb') + const rec = recorder() + + await replayFragments({ query: rec.query, modules: freshLoader(tmpRoot) }) + + assert.deepEqual(rec.ran, []) +}) + +test('fragments are split exactly the way core schema.sql is', async () => { + // §2.6's "split the same way" is a promise about shared code, so the thing + // worth asserting is that the module path produces what the shared splitter + // produces — including the trailing-comment case that would otherwise chop a + // statement in half at the `;` inside it. + const sql = [ + '-- a leading comment block', + '-- with two lines; and a semicolon in it', + 'CREATE TABLE IF NOT EXISTS beta_a (id INT); -- trailing; comment', + '', + 'CREATE TABLE IF NOT EXISTS beta_b (id INT);', + ].join('\n') + writeModule('beta', { schema: sql }) + const rec = recorder() + + await replayFragments({ query: rec.query, modules: freshLoader(tmpRoot) }) + + assert.deepEqual(rec.ran, splitStatements(sql)) + assert.equal(rec.ran.length, 2) +}) + +// ── Failure is a state ───────────────────────────────────────────────────── + +test('a fragment that throws fails its own module and no one else', async () => { + writeModule('aaa', { schema: 'CREATE TABLE IF NOT EXISTS aaa_x (id INT);' }) + writeModule('bbb', { schema: 'CREATE TABLE IF NOT EXISTS bbb_boom (id INT);' }) + writeModule('ccc', { schema: 'CREATE TABLE IF NOT EXISTS ccc_x (id INT);' }) + const loader = freshLoader(tmpRoot) + const rec = recorder('bbb_boom') + + // Never throws — this is called on the boot path, between core's schema and + // seedDefaults(), and one bad module must not stop the site coming up. + await replayFragments({ query: rec.query, modules: loader }) + + assert.equal(stateOf(loader, 'aaa').state, 'registered') + assert.equal(stateOf(loader, 'bbb').state, 'startup_failed') + assert.match(stateOf(loader, 'bbb').reason, /ER_PARSE_ERROR/) + // The one that matters: the module AFTER the failure still got its tables. + assert.equal(stateOf(loader, 'ccc').state, 'registered') + assert.equal(rec.ran.length, 3) +}) + +test('a fragment stops at its first failing statement', async () => { + writeModule('part', { + schema: [ + 'CREATE TABLE IF NOT EXISTS part_a (id INT);', + 'CREATE TABLE IF NOT EXISTS part_bad (id INT);', + 'CREATE TABLE IF NOT EXISTS part_c (id INT);', + ].join('\n'), + }) + const loader = freshLoader(tmpRoot) + const rec = recorder('part_bad') + + await replayFragments({ query: rec.query, modules: loader }) + + // Two attempted, the third never reached. The first table survives, and is + // accepted rather than compensated for: DDL self-commits in MariaDB, so no + // transaction could roll it back, and §2.6's idempotence rule is what makes + // re-running the corrected fragment safe. + assert.equal(rec.ran.length, 2) + assert.equal(stateOf(loader, 'part').state, 'startup_failed') +}) + +test('a module whose fragment failed keeps its URLs and answers 503', async () => { + // §4.4's right-hand column, now reachable for real rather than by a hand-moved + // state: schema replay is the first thing in the lifecycle that fails AFTER + // the routes are already mounted. + writeModule('svc', { + schema: 'CREATE TABLE IF NOT EXISTS svc_boom (id INT);', + mounts: { public: ['/widgets'] }, + }) + const tiers = emptyTiers() + const loader = freshLoader(tmpRoot, tiers) + const app = await startApp((a) => a.use('/public', tiers.public)) + + try { + assert.equal((await fetch(`${app.url}/public/widgets`)).status, 200) + + await replayFragments({ query: recorder('svc_boom').query, modules: loader }) + + assert.equal((await fetch(`${app.url}/public/widgets`)).status, 503) + } finally { + await app.close() + } +}) + +test('a module that failed validation is not replayed at all', async () => { + // It is never going to run, so creating its tables would leave an operator + // with rows belonging to a module that does not load. + writeModule('bad', { schema: 'CREATE TABLE IF NOT EXISTS not_prefixed (id INT);' }) + writeModule('good', { schema: 'CREATE TABLE IF NOT EXISTS good_x (id INT);' }) + const loader = freshLoader(tmpRoot) + const rec = recorder() + + await replayFragments({ query: rec.query, modules: loader }) + + assert.equal(stateOf(loader, 'bad').state, 'startup_failed') + assert.equal(rec.ran.length, 1) + assert.match(rec.ran[0], /good_x/) +}) + +// ── The seed script ──────────────────────────────────────────────────────── + +test('replay is skipped, not thrown, when no scan happened in this process', async () => { + // `npm run seed` (db/seed.js) calls ensureSchema() standalone without ever + // requiring app.js, so the loader never ran. Before this was handled it was + // fragments()'s §7.6 throw, which would have broken seeding outright. + process.env.MODULES_DIR = tmpRoot + delete require.cache[require.resolve('../src/modules/loader')] + // eslint-disable-next-line global-require + const loader = require('../src/modules/loader') + const rec = recorder() + + await replayFragments({ query: rec.query, modules: loader }) + + assert.deepEqual(rec.ran, []) + assert.equal(loader.isLoaded(), false) +}) -- 2.49.1 From 6195c76d6148ccee79b2a1eaae6e01f9d579074b Mon Sep 17 00:00:00 2001 From: wtclaude Date: Mon, 10 Aug 2026 17:47:59 -0500 Subject: [PATCH 04/30] feat(modules): the three de-entanglement registries, with core as the registrant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2 PR 4 of docs/website/MODULE_SYSTEM.md §2.7. Adds server/src/modules/registries.js and moves core's own notification streams, announce leg and users-detail routes behind it, so the three seams §1.8 and §1.9 named are exercised on every boot before any module depends on them. Registering is validate-then-commit per registrant: the loader stages what a module claims and the second pass commits it, so a module that throws halfway through register() — or fails checkDeclared after it — leaves nothing behind. That is the registry-side twin of PR 2's second-pass mount rule. Four decisions, all the recommended option: - announce legs became a child table. `announce_job_legs` replaces the towncrier_*/discord_* column groups, so the leg set is data: core registers `discord`, module-uo will register `towncrier`, and a module cannot ALTER a core table to add its own. Backfill is guarded on information_schema (a SELECT of a dropped column is a parse error, not a runtime one) and the columns go with DROP COLUMN IF EXISTS. Verified against the live dev DB: three legacy jobs migrated faithfully, three replays, no duplicates. - `mapEvent` dropped from registerNotificationStreams. §1.8 already inverts the push path so a module owns fromShardEvent and calls core's publish() with a stream id it resolved; a second mapping mechanism was a leftover. The public safety filter, the kinds it reads and the streams it protects now live in one file and move together. - core registers through the same staging area a module uses, via an explicit registries.registerCore() in app.js before modules.load(). - core's six /admin/users/:id/shard/* paths now go through the `admin.users.detail` slot, and getUser moved back to admin.controller.js. Found on the way, and the reason two build tools changed: - scripts/routeManifest.js could not decode a parameterised mount. Its unwinder expected `(?:([^\/]+?))`; express 4.22 emits `(?:\/([^/]+?))` with the separator inside the group. The branch had never run. It threw rather than guessing, which is what it is for. - swagger-autogen cannot follow a route into an extension slot — the slot's router is created by declareSlot() and filled later, so there is no literal mount for a static parse. Regenerating deleted 407 lines and printed `Swagger-autogen: Success`, the spike's exact failure (MODULE_API.md §7.4). swagger/slotSpecs.js generates a fragment per filled slot and re-roots it at the prefix the router actually hangs at in the live app — read from the express stack via routeManifest's own mountPath, so the manifest and the spec cannot disagree. swagger/mergeSpec.js is the merge helper core owes for module fragments anyway (§6.1a), proved here against core's own slot first. 884 tests pass (856 before). routes.manifest.json is unchanged at 229 routes. The OpenAPI spec diff is two lines of intent: the retry endpoint's summary, and its `leg` no longer being a fixed enum. Co-Authored-By: Claude --- client/src/routes/admin/views/PostEditor.jsx | 23 +- server/db/schema.sql | 86 +++-- server/routes.guards.json | 2 +- server/scripts/routeManifest.js | 19 +- server/src/app.js | 7 + server/src/config/coreStreams.js | 26 ++ ...notificationStreams.js => shardStreams.js} | 36 +- .../src/model/announceJobs/announceJobs.db.js | 114 ++++-- .../model/announceJobs/announceJobs.logic.js | 120 ++---- .../model/announceJobs/announceJobs.model.js | 55 ++- .../notificationSubs.model.js | 6 +- server/src/modules/loader.js | 52 ++- server/src/modules/registries.js | 356 ++++++++++++++++++ .../src/router/v1/admin/admin.controller.js | 18 + server/src/router/v1/admin/posts.router.js | 12 +- server/src/router/v1/admin/users.router.js | 108 +----- .../router/v1/admin/usersShard.controller.js | 14 +- .../src/router/v1/admin/usersShard.router.js | 111 ++++++ .../v1/auth/notifications.controller.js | 8 +- server/src/utils/announceWorker.js | 96 ++--- server/src/utils/discordAnnounce.js | 45 +++ server/src/utils/pushDispatch.js | 44 +-- server/src/utils/shardAnnounce.js | 77 ++++ server/src/utils/shardIngest.js | 4 +- server/src/utils/shardPush.js | 47 +++ server/swagger/mergeSpec.js | 115 ++++++ server/swagger/slotSpecs.js | 120 ++++++ server/swagger/swagger-output.json | 7 +- server/swagger/swagger.js | 34 +- server/test/adminUserShard.test.js | 13 +- server/test/adminUsers.test.js | 65 ++++ server/test/announceJobs.test.js | 65 ++-- server/test/announceLegs.test.js | 206 ++++++++++ server/test/moduleLoader.test.js | 75 +++- server/test/moduleRegistries.test.js | 216 +++++++++++ server/test/pushDispatch.test.js | 11 +- 36 files changed, 1948 insertions(+), 465 deletions(-) create mode 100644 server/src/config/coreStreams.js rename server/src/config/{notificationStreams.js => shardStreams.js} (81%) create mode 100644 server/src/modules/registries.js create mode 100644 server/src/router/v1/admin/usersShard.router.js create mode 100644 server/src/utils/discordAnnounce.js create mode 100644 server/src/utils/shardAnnounce.js create mode 100644 server/src/utils/shardPush.js create mode 100644 server/swagger/mergeSpec.js create mode 100644 server/swagger/slotSpecs.js create mode 100644 server/test/adminUsers.test.js create mode 100644 server/test/announceLegs.test.js create mode 100644 server/test/moduleRegistries.test.js diff --git a/client/src/routes/admin/views/PostEditor.jsx b/client/src/routes/admin/views/PostEditor.jsx index fb7e68c..61b90b5 100644 --- a/client/src/routes/admin/views/PostEditor.jsx +++ b/client/src/routes/admin/views/PostEditor.jsx @@ -177,14 +177,15 @@ const delStyle = { } // ── Announcement status panel ──────────────────────────────────────────────── -// Shows the town-crier + Discord delivery state for a published news post and -// offers a per-leg retry (useful after fixing the sidecar / news channel without -// re-publishing). Only rendered for news posts in edit mode; renders nothing -// until the post has actually been announced (no job row yet → nothing to show). -const LEG_META = { - towncrier: { label: 'In-game town crier' }, - discord: { label: 'Discord #news' }, -} +// Shows each delivery leg's state for a published news post and offers a per-leg +// retry (useful after fixing the sidecar / news channel without re-publishing). +// Only rendered for news posts in edit mode; renders nothing until the post has +// actually been announced (no job row yet → nothing to show). +// +// The legs and their labels come from the JOB, not from a constant here: which +// legs exist is decided by what the server has registered, so an installed module +// brings its own leg and this panel renders it with no client change +// (docs/website/MODULE_SYSTEM.md §1.8). const STATUS_STYLE = { done: { color: '#7bbf8f', label: 'delivered' }, pending: { color: '#d9b84a', label: 'pending' }, @@ -227,14 +228,12 @@ function AnnouncePanel({ postId }) { return (
Announcement - {['towncrier', 'discord'].map((leg) => { - const status = job[`${leg}_status`] - const err = job[`${leg}_last_error`] + {(job.legs || []).map(({ leg, label, status, last_error: err }) => { const s = STATUS_STYLE[status] || STATUS_STYLE.pending return (
- {LEG_META[leg].label} + {label} ● {s.label} {status !== 'done' && (