feat(template): a module that builds and loads — Phase 5 slice 1
The kit's `template/`: a complete, minimal Runic Gateway module a reader copies,
renames, and runs before reading a chapter. Slice 0 landed the workflow that runs
it; this is the tree that workflow was written against, so the `template` job
arms itself with no edit to the guard.
Installed into a real core it adds one public page at `/examplegame/status`, a nav
row pointing at it, one API route described in an OpenAPI fragment core merges,
one table created by an idempotent schema fragment and dropped by a purge file,
and both lifecycle hooks. That is deliberately less than a real module does; what
it is complete about is the shape — every seam used once, with the reasoning next
to it.
Four decisions, settled with the org lead:
1. **Public tier only, plus the lifecycle hooks.** §2.11.1 d1's "one public route",
plus enough to show the whole vertical seam once. Admin and player tiers become
worked examples quoted from module-uo in chapter 2 rather than two thirds of a
tree the reader deletes on day one.
2. **The release workflow ships as a file, in BOTH flavours** — `.gitea/` and
`.github/`. Neither runs where it sits (a workflow is only read from a
repository root) and each arms itself when the reader's copy is its own repo.
Packaging is the part of a module that cannot be guessed at, and the kit's
audience is outside this org, so assuming Gitea would have been assuming our
own deployment. Core installs from a URL and does not care where the release
lives — only that the host is on the operator's `MODULE_SOURCE_HOSTS`.
3. **A rename checklist that CI verifies**, not a rename script. `template/README.md`
carries the table; `scripts/checkRenameSites.js` holds it against the tree in
both directions — an unlisted file that still carries the placeholder fails, and
so does a listed file that no longer does. The second half is the one usually
left out and the more valuable: a row that has stopped matching reads as
instructions to edit something that is not there. Same rule core's identifier
check follows about its own exemptions. It has its own ten-test suite, run by
CI as `node --test`, because a check that has never been shown to fail is a
check nobody knows the state of.
4. **A neutral invented game.** One deviation from the literal answer, forced by
decision 3: the id is `examplegame`, not `example`. The checklist check is a
text search, and `example` occurs in ordinary English ("for example") all over
prose that is not a rename site — a placeholder that cannot occur by accident is
what makes the check answerable instead of a source of false alarms someone
learns to ignore.
**The pin moves to the 1.4.0 bump** (website `edge` 1b692bf), which is what
`template/module.json` declares as `coreApi`. Slice 0 pinned its parent, before
1.4.0 existed, so `checkCoreApi.js` arms for the first time here — it asserts
EQUALITY, and its failing on the next contract bump is the system working.
Also in CI: the client tests now run AFTER the build (two of them read the built
chunk and skip without one — run first, the job reports green while asking nothing
about the artifact that ships), and `check:swagger` verifies the committed
fragment is current.
## The finding: an UPDATE that changes nothing does not touch ON UPDATE CURRENT_TIMESTAMP
Every suite passed, both guards passed, the chunk built, the module loaded into a
real core and the page rendered correctly. Two hours later the same page said the
world was offline, and it was wrong.
`updated_at` was declared `ON UPDATE CURRENT_TIMESTAMP`, and MariaDB fires that
only when an UPDATE actually CHANGES a value. The boot refresh writes the same
numbers every thirty seconds — which is exactly what a quiet game looks like — so
the timestamp froze at the first write, the row crossed the freshness window, and
the model correctly reported a stale row as offline. Verified against the live
database: two hours of refreshes, `updated_at` still the boot timestamp.
No test in this repo could see it. The model takes its clock as an argument, and
nothing in a suite runs the same UPDATE twice against a real database. It is only
visible as a page that was right when you looked at it and wrong an hour later.
The writer now sets `updated_at = CURRENT_TIMESTAMP` explicitly and the column
drops the clause that was not doing what it looked like it was doing; both carry
the reasoning. Re-verified end to end: the timestamp advances every interval and
the API reports fresh.
Falling out of the fix, the schema fragment gained the rule the reader hits next:
**changing a table is an ALTER, never an edit to its CREATE** — `CREATE TABLE IF
NOT EXISTS` does nothing when the table exists, so an edited column definition
takes effect on a fresh install and on no existing one, which is the worst
possible split because your development database is usually the fresh one.
## Verified
- 29 server tests, 18 client tests, 10 kit-script tests; `check:imports`,
`check:externals`, `check:swagger` and `checkCoreApi` all green, run in CI's own
order from a clean `npm ci`.
- Browser smoke (MODULE_API.md §7.7) against a real core built from the pinned
ref: module `started`, published on `/api/v1/public/modules`, chunk served
`no-cache` with the right MIME from the entry's directory while `module.json`
and the server source 404, script tag injected after core's bundle, the page
rendering inside core's own chrome, the nav row interleaved into the public
header between Wiki and About, SPA navigation into it from another page, the
module's path and schema and tag merged into `/api/docs.json`, and
`[examplegame] registered against core API 1.4.0` in the console with no CSP
report and no React error.
Refs: MODULE_SYSTEM.md §2.11.1 (slice 1), MODULE_API.md §2.x, §3.x, §5.1, §7.7.
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
190
template/server/scripts/checkImports.js
Normal file
190
template/server/scripts/checkImports.js
Normal file
@@ -0,0 +1,190 @@
|
||||
#!/usr/bin/env node
|
||||
// ── §5.1 — zero internal imports ───────────────────────────────────────────
|
||||
//
|
||||
// The acceptance test for the whole module contract. A module that reaches into
|
||||
// core's tree still works — right up until core moves a file — and the boundary
|
||||
// this workstream exists to build is worth exactly as much as this check is.
|
||||
//
|
||||
// MODULE_API.md §5.1 sketches it as a grep for `../../`. That is the shape of
|
||||
// the violation but not the rule, and the difference matters in both directions:
|
||||
// a grep says nothing about `require('../../../../etc/passwd')` from a deeply
|
||||
// nested file (which it catches by accident) and false-alarms on a legitimate
|
||||
// `require('../module.json')` from `server/` (which it catches wrongly). So this
|
||||
// RESOLVES each specifier against the file that wrote it and asks whether the
|
||||
// result is still inside the module root — the actual rule, stated once.
|
||||
//
|
||||
// Bare specifiers are checked too, and against a stricter list than "is it
|
||||
// installed": core hands the module express, express-validator, the database and
|
||||
// the logger on `ctx` precisely so the module never resolves them, and Node's
|
||||
// resolver cannot reach core's `node_modules` from here anyway. A bare
|
||||
// `require` that is not a Node builtin is therefore a module that will fail to
|
||||
// load on a real install, with a message about a missing package rather than
|
||||
// about the rule it broke.
|
||||
//
|
||||
// **That second check applies to SHIPPED code only.** `test/` and `scripts/`
|
||||
// never run inside core's process — the fakes in `test/_fakes.js` build a real
|
||||
// `express` router precisely so the module's routers are exercised for real —
|
||||
// so they may use devDependencies. The containment check applies everywhere,
|
||||
// because a test that reaches into core's tree is a test that passes on this
|
||||
// machine and nowhere else.
|
||||
//
|
||||
// Run over the SERVER half. The client half's equivalents are its Vite build,
|
||||
// which fails if a shared dependency resolves into node_modules, and
|
||||
// client/scripts/checkExternals.js, which asks the built chunk whether any bare
|
||||
// specifier survived.
|
||||
|
||||
const fs = require('fs')
|
||||
const path = require('path')
|
||||
// Node's own answer, not a list reconstructed from `builtinModules`. That list
|
||||
// omits `test` on Node 20 and includes it on Node 24, so a suite that requires
|
||||
// `node:test` passed locally and failed in CI on the very first run — reported
|
||||
// as the module boundary being broken, which it was not. `isBuiltin` is the
|
||||
// authoritative check and handles the `node:` prefix itself.
|
||||
const { isBuiltin } = require('module')
|
||||
|
||||
const MODULE_ROOT = path.resolve(__dirname, '..', '..')
|
||||
const SERVER_ROOT = path.join(MODULE_ROOT, 'server')
|
||||
|
||||
// Packages the SHIPPED half may resolve for itself: this package's declared
|
||||
// `dependencies`, and nothing else. Read from package.json rather than listed
|
||||
// here, so adding one is a visible, reviewable edit to the manifest that also
|
||||
// changes what CI installs and what the release tarball carries.
|
||||
//
|
||||
// Adding a dependency is a real decision. §2.7 permits a module its own, and the
|
||||
// release tarball carries `server/node_modules` because an operator never builds
|
||||
// — so every entry is weight in the artifact and a package the operator's
|
||||
// deployment now runs. Anything core already owns must come from `ctx` instead:
|
||||
// a second express is a second Router prototype, a second express-rate-limit is
|
||||
// a second store, and a limit enforced by two independent counters is not the
|
||||
// limit either of them states.
|
||||
|
||||
const SKIP_DIRS = new Set(['node_modules', 'coverage', '.git'])
|
||||
|
||||
// Directories whose contents never run inside core's process, and may therefore
|
||||
// resolve this package's devDependencies.
|
||||
const NOT_SHIPPED = [path.join(SERVER_ROOT, 'test'), path.join(SERVER_ROOT, 'scripts')]
|
||||
const isShipped = (file) => !NOT_SHIPPED.some((d) => file.startsWith(d + path.sep))
|
||||
|
||||
const manifest = JSON.parse(fs.readFileSync(path.join(SERVER_ROOT, 'package.json'), 'utf8'))
|
||||
const dependencies = new Set(Object.keys(manifest.dependencies || {}))
|
||||
const devDependencies = new Set(Object.keys(manifest.devDependencies || {}))
|
||||
|
||||
// `require('x')`, `from 'x'`, `import('x')`. Deliberately textual: parsing would
|
||||
// need a dependency, and a specifier this pattern misses is a specifier written
|
||||
// to be missed, which review catches and a stricter regexp would not.
|
||||
const SPECIFIER = /(?:require\(|from\s+|import\()\s*['"]([^'"]+)['"]/g
|
||||
|
||||
/**
|
||||
* Blank out comments and template literals before scanning.
|
||||
*
|
||||
* Not a nicety — without it this file fails on ITSELF, because the comments
|
||||
* above name `require('../../../../etc/passwd')` as an example of what to
|
||||
* catch, and index.js explains in prose why it must never `require('express')`.
|
||||
* A boundary check that cannot survive being described is a check people stop
|
||||
* writing comments around.
|
||||
*
|
||||
* A character walk rather than a regexp, because the two get in each other's
|
||||
* way: `'https://x'` contains a line-comment opener inside a string, and
|
||||
* `// don't` contains a quote inside a comment. Tracking the state is shorter
|
||||
* than the regexp that would almost handle it. Content is replaced with spaces
|
||||
* rather than removed so nothing else has to care.
|
||||
*/
|
||||
function stripCommentsAndTemplates(src) {
|
||||
let out = ''
|
||||
let i = 0
|
||||
const keep = (n) => { out += src.slice(i, i + n); i += n }
|
||||
const blank = (end) => { out += src.slice(i, end).replace(/[^\n]/g, ' '); i = end }
|
||||
while (i < src.length) {
|
||||
const two = src.slice(i, i + 2)
|
||||
if (two === '//') {
|
||||
const nl = src.indexOf('\n', i)
|
||||
blank(nl === -1 ? src.length : nl)
|
||||
} else if (two === '/*') {
|
||||
const end = src.indexOf('*/', i + 2)
|
||||
blank(end === -1 ? src.length : end + 2)
|
||||
} else if (src[i] === '"' || src[i] === "'") {
|
||||
// Strings are KEPT — they are where the specifiers live.
|
||||
const quote = src[i]
|
||||
keep(1)
|
||||
while (i < src.length && src[i] !== quote) keep(src[i] === '\\' ? 2 : 1)
|
||||
keep(1)
|
||||
} else if (src[i] === '`') {
|
||||
// Template literals are blanked: nothing may `require` a template, and a
|
||||
// template holding SQL or HTML is a rich source of false positives.
|
||||
i += 1
|
||||
out += ' '
|
||||
while (i < src.length && src[i] !== '`') {
|
||||
if (src[i] === '\\') { out += ' '; i += 2 } else { out += src[i] === '\n' ? '\n' : ' '; i += 1 }
|
||||
}
|
||||
i += 1
|
||||
out += ' '
|
||||
} else {
|
||||
keep(1)
|
||||
}
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
function* walk(dir) {
|
||||
for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
|
||||
if (entry.isDirectory()) {
|
||||
if (!SKIP_DIRS.has(entry.name)) yield* walk(path.join(dir, entry.name))
|
||||
} else if (/\.(js|mjs|cjs)$/.test(entry.name)) {
|
||||
yield path.join(dir, entry.name)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Every boundary violation under `root`, resolved against `moduleRoot`.
|
||||
*
|
||||
* Exported so `test/checkImports.test.js` can point it at fixtures. A check that
|
||||
* has never been shown to fail is a check nobody knows the state of — and this
|
||||
* one guards the acceptance criterion for the whole contract.
|
||||
*/
|
||||
function scan(root, moduleRoot = MODULE_ROOT, { shipped = isShipped, deps = dependencies, dev = devDependencies } = {}) {
|
||||
const violations = []
|
||||
for (const file of walk(root)) {
|
||||
const source = stripCommentsAndTemplates(fs.readFileSync(file, 'utf8'))
|
||||
for (const [, specifier] of source.matchAll(SPECIFIER)) {
|
||||
if (specifier.startsWith('.')) {
|
||||
const resolved = path.resolve(path.dirname(file), specifier)
|
||||
if (resolved !== moduleRoot && !resolved.startsWith(moduleRoot + path.sep)) {
|
||||
violations.push({ file, specifier, why: 'escapes the module root' })
|
||||
}
|
||||
} else if (path.isAbsolute(specifier)) {
|
||||
violations.push({ file, specifier, why: 'absolute path' })
|
||||
} else {
|
||||
const pkg = specifier.startsWith('@')
|
||||
? specifier.split('/').slice(0, 2).join('/')
|
||||
: specifier.split('/')[0]
|
||||
const allowed = deps.has(pkg) || (!shipped(file) && dev.has(pkg))
|
||||
// The `node:` prefix can only ever name a builtin, so it never reaches
|
||||
// node_modules and is safe whatever this Node version enumerates.
|
||||
const builtin = isBuiltin(specifier) || specifier.startsWith('node:')
|
||||
if (!builtin && !allowed) {
|
||||
violations.push({ file, specifier, why: 'undeclared bare specifier — should this come from ctx?' })
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
return violations
|
||||
}
|
||||
|
||||
module.exports = { scan, stripCommentsAndTemplates, SERVER_ROOT, MODULE_ROOT }
|
||||
|
||||
// Required by a test, or run as the check? Only the second one exits.
|
||||
if (require.main !== module) return
|
||||
|
||||
const violations = scan(SERVER_ROOT)
|
||||
|
||||
if (violations.length) {
|
||||
console.error(`\n${violations.length} import(s) break the module boundary (MODULE_API.md §5.1):\n`)
|
||||
for (const v of violations) {
|
||||
console.error(` ${path.relative(MODULE_ROOT, v.file)}\n "${v.specifier}" — ${v.why}`)
|
||||
}
|
||||
console.error('')
|
||||
process.exit(1)
|
||||
}
|
||||
|
||||
console.log(`OK — no import escapes the module root (${SERVER_ROOT}).`)
|
||||
Reference in New Issue
Block a user