From 63e6c2b5d12b39647909e8d18ba067cb0236dd26 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Mon, 10 Aug 2026 16:58:11 -0500 Subject: [PATCH] docs(website): settle how module schema fragments are validated and replayed Docs half of website PR 3 (phase 2). MODULE_SYSTEM.md 2.7 gains the PR 3 entry; MODULE_API.md 2.6 gains the decisions the section did not previously answer. The one that shapes the code: a fragment is VALIDATED at load time and EXECUTED later, split on whether a database is needed to know the answer. Everything 2.6 states about the SQL is knowable by reading the file, so breaking one of those rules costs a module its mount entirely rather than mounting and 503ing with its tables half created. What is left for the replay is the class of failure only the server can report, and that is post-mount and answers 503. Also recorded: the rules are enforced as a leading-verb allowlist (CREATE, ALTER, INSERT, UPDATE -- the four core's own schema.sql uses) rather than the DROP denylist 2.6 words them as, because the file is replayed on every boot and a denylist only ever bans what somebody thought of; the replay sits outside ensureSchema's wait-for-the-database retry loop, so one module's bad SQL cannot cost the boot two minutes; partial application is accepted, since MariaDB self-commits DDL and no transaction could undo it; and `npm run seed` is the one sanctioned caller that replays nothing, because it never requires app.js and so has no scan to read. Co-Authored-By: Claude --- website/MODULE_API.md | 34 ++++++++++++++++++++++++++++++++++ website/MODULE_SYSTEM.md | 16 ++++++++++++---- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/website/MODULE_API.md b/website/MODULE_API.md index 450a99b..25b3010 100644 --- a/website/MODULE_API.md +++ b/website/MODULE_API.md @@ -237,6 +237,40 @@ immediately after it, statement by statement, split the same way. It is subject core's file already follows: `CREATE TABLE IF NOT EXISTS`, `ALTER TABLE … ADD COLUMN IF NOT EXISTS`, no `--` inside a string literal, no `DROP`. +"Split the same way" is shared code, not a shared description: `utils/sqlStatements.js` holds the +splitter and both callers use it. It is its own file rather than an export of `utils/db.js` because +the loader validates fragments at require time and must not pull the mariadb pool into `app.js`'s +require chain to do it. + +**The rules above are enforced at LOAD time, not at replay time** (PR 3). Everything §2.6 states +about the SQL is knowable by reading the file, so a fragment that breaks a rule costs the module its +mount entirely (§4.4's left-hand column) rather than mounting and then 503ing with tables half +created. What is left for the replay is the class of failure only the database can report — an +unknown column type, a bad foreign key — and those are post-mount and answer 503. + +**The check is a leading-verb allowlist: `CREATE`, `ALTER`, `INSERT`, `UPDATE`.** Those are the four +core's own `schema.sql` uses. It is an allowlist rather than the `DROP` denylist this section words +it as because a fragment is **replayed on every boot**: `TRUNCATE` and `DELETE` would empty a table +at every restart, `RENAME` would fail at the second one, and `GRANT`/`SET`/`USE` are core's business. +A denylist only ever bans what somebody thought of. It is a leading-verb check and claims no more: +`ALTER TABLE x DROP COLUMN y` passes it, and catching that needs a SQL parser — a large dependency +for a rule whose job is stopping the obvious foot-gun early. A `CREATE TABLE` missing `IF NOT EXISTS` +is rejected on the same grounds: it succeeds exactly once and fails every boot after, presenting to +an operator as a module that broke on restart. + +**The replay is outside `ensureSchema()`'s retry loop.** Core's schema is retried ten times while the +database comes up; a fragment that throws is one module's failure, not a signal the database is not +ready, and retrying core's whole schema over one module's bad SQL would turn a 503'd module into a +two-minute boot. Partial application is accepted rather than compensated for — MariaDB self-commits +each DDL statement, so no transaction could roll back the tables created before the failing one, and +the idempotence rule is what makes re-running a corrected fragment safe. + +**One caller replays nothing, deliberately.** `db/seed.js` (`npm run seed`) calls `ensureSchema()` +standalone without requiring `app.js`, so no scan has happened and `fragments()`'s §7.6 throw would +break seeding outright. The replay asks `isLoaded()` and logs the skip. That is the only sanctioned +use of that predicate: everywhere else, reading the module list before `load()` still throws, because +a booting server quietly getting no module tables is precisely what §7.6 exists to prevent. + **Table names are namespaced and collision-checked.** New tables must be prefixed `_`. The loader extracts every `CREATE TABLE IF NOT EXISTS ` from the fragment and rejects the module if a name collides with a core table or with another module's — a wrong `DROP`-free fragment can diff --git a/website/MODULE_SYSTEM.md b/website/MODULE_SYSTEM.md index d2f0d5b..33d7194 100644 --- a/website/MODULE_SYSTEM.md +++ b/website/MODULE_SYSTEM.md @@ -453,7 +453,7 @@ too (API §7.2). Exit criterion: `routes.manifest.json` diff is zero lines and every existing test passes. If Phase 2 changes one URL, it is wrong. -**Progress: PRs 1-2 done.** +**Progress: PRs 1-3 done.** - **PR 1** — `installed_modules` and the state machine, with the stored shape and the boot rules settled in §2.4 above. @@ -466,10 +466,18 @@ changes one URL, it is wrong. de-entanglement registries and the two lifecycle hooks 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. 28 tests, all on the failure paths. +- **PR 3** — schema fragment replay. `ensureSchema()` replays each installed module's fragment after + core's, with the statement splitter extracted to `utils/sqlStatements.js` so both are split by the + same code. The decision that shaped it, recorded in [`MODULE_API.md`](MODULE_API.md) §2.6: the + fragment is **validated at load time and executed later**, split on whether a database is needed to + know the answer — a fragment breaking a stated rule never mounts, while a failure only the server + could report (a bad column type) is post-mount and 503s. The rules are enforced as a **leading-verb + allowlist** (`CREATE`, `ALTER`, `INSERT`, `UPDATE`) rather than the `DROP` denylist §2.6 words them + as, because the file is replayed on **every boot**. Found while wiring it: `npm run seed` calls + `ensureSchema()` without ever requiring `app.js`, so the replay has to tolerate an unscanned loader. -There is still no module on the volume, no schema replay and no boot wiring, so this changes nothing -an operator or a client can see: 842 tests pass and `routes.manifest.json` is unchanged at 229 -routes. +There is still no module on the volume and no boot wiring, so this changes nothing an operator or a +client can see: 856 tests pass and `routes.manifest.json` is unchanged at 229 routes. **Phase 3 — Extract `module-uo`.** Moves out of `website/`: the 8 model directories and their 25 tables; the nine UO `utils/` files plus `newsGump.js`; the 13 router/controller files; -- 2.49.1