fix(core): the last fixes' core half (W1 W2 W5 W19)
All checks were successful
PR Checks / bot-tests (pull_request) Successful in 49s
PR Checks / client-build (pull_request) Successful in 1m4s
PR Checks / server-tests (pull_request) Successful in 6m6s

Stage 4 of docs/modules/rust/PLAN_LAST_FIXES.md, with the org lead's
2026-10-22 decisions.

W19: one module per site at boot, as at install. The first module
directory by name holds the site, whether or not it then loads; every
other is recorded startup_failed at the manifest stage, before a line of
its code is required, with a reason naming the module that holds the
site. Module-uo and Module-Rust side by side used to fail the second
with a slot collision. Loader, lifecycle, schema and event-contract
tests that loaded several fixture modules now load each case alone.

W1: a reconcile files a resource the game no longer has, and that is
past its `until`, as expired rather than orphaned, with the same log
line expired() writes. No MODULE_API bump: `until` and reconcile's
answer are unchanged.

W5: a dry run that passes on the draft is kept with the fingerprint of
the validated spec (three nullable columns on event_definitions).
Publish stamps the new version verified, with that dry run's reviewer
and time, when the spec it freezes has the same fingerprint, and writes
version.verified on the occurrences it re-pins. Verify answers
draftRecorded and publish answers verified; OpenAPI regenerated.

W2: the run console says "Phase p1: steps done, waiting to advance" and
"Why this phase has not advanced", not "has not started".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E14m6SuuY6i1vASFeGDBeY
This commit is contained in:
2026-10-22 00:38:34 -05:00
parent 9a1654fc54
commit f40dd765c3
18 changed files with 632 additions and 210 deletions

View File

@@ -95,6 +95,17 @@ const oneRoute = (prefix, tier = 'public') => ({
const stateOf = (loader, id) => loader.list().find((m) => m.id === id)
/**
* One module in a modules directory of its own, loaded. A site runs one module
* (W19), so a test that checks several cases loads each case by itself rather
* than side by side, where every one after the first would be refused.
*/
function alone(id, spec, tiers) {
tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'rg-modules-'))
writeModule(id, spec)
return freshLoader(tmpRoot, tiers)
}
beforeEach(() => {
tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'rg-modules-'))
})
@@ -140,23 +151,65 @@ test('load() is once-only, so a second call cannot double-mount', () => {
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 = () => {}' })
// ── One module per site (W19) ──────────────────────────────────────────────
test('the first module by name holds the site; every other is refused unrequired', () => {
// Install refuses a second module, so this is the hand-copied case: Module-uo
// and Module-Rust both fill `admin.users.detail`, and the second used to fail
// with a slot collision that named neither the rule nor the fix.
writeModule('ccc', { server: 'throw new Error("ccc should never be required")' })
writeModule('aaa', { server: 'module.exports = () => {}' })
writeModule('bbb', { manifest: { version: '2.1.0' }, server: 'throw new Error("bbb should never be required")' })
const loader = freshLoader(tmpRoot)
assert.deepEqual(loader.list().map((m) => m.id), ['aaa', 'bbb', 'ccc'])
assert.equal(stateOf(loader, 'aaa').state, 'registered')
for (const id of ['bbb', 'ccc']) {
const refused = stateOf(loader, id)
assert.equal(refused.state, 'startup_failed')
assert.equal(refused.stage, 'manifest')
assert.match(refused.reason, /already runs the module "aaa", and a site runs one module/)
assert.match(refused.reason, new RegExp(`Remove "${id}" or "aaa"`))
}
// The version still reaches the admin panel, read from module.json alone.
assert.equal(stateOf(loader, 'bbb').version, '2.1.0')
})
test('the first module holds the site even when it fails to load', () => {
// Which module a site runs is decided by the directory, not by which one
// happened to start: a broken first module does not hand the site to the
// second, where an operator would be debugging a module they did not mean to run.
writeModule('aaa', { server: 'throw new Error("boom")' })
writeModule('bbb', { server: 'module.exports = () => {}' })
const loader = freshLoader(tmpRoot)
const bad = stateOf(loader, 'aaa')
assert.equal(bad.state, 'startup_failed')
assert.match(bad.reason, /boom/)
assert.match(stateOf(loader, 'bbb').reason, /already runs the module "aaa"/)
})
test('a second module never mounts, registers or claims a table', () => {
const tiers = emptyTiers()
writeModule('aaa', oneRoute('/thing'))
writeModule('bbb', {
...oneRoute('/other'),
schema: 'CREATE TABLE IF NOT EXISTS bbb_x (id INT);',
})
const loader = freshLoader(tmpRoot, tiers)
const claims = (prefix) =>
tiers.public.stack.some((l) => l.regexp && !l.regexp.fast_slash && l.match(prefix))
assert.equal(claims('/thing'), true)
assert.equal(claims('/other'), false)
assert.deepEqual(loader.fragments().map((f) => f.id), [])
assert.deepEqual(loader.bootable().map((b) => b.id), ['aaa'])
})
// ── 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')
test('a module whose entry point throws is recorded, and the site still comes up', () => {
const loader = alone('bbb', { server: 'throw new Error("boom")' })
const bad = stateOf(loader, 'bbb')
assert.equal(bad.state, 'startup_failed')
assert.match(bad.reason, /boom/)
@@ -204,15 +257,6 @@ test('an unknown extension slot is rejected; only core may declare a slot', () =
// ── 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
@@ -226,41 +270,41 @@ test('a module cannot take a prefix core owns, asked of the live tier routers',
}
/* eslint-enable global-require */
writeModule('greedy', { manifest: { mounts: { admin: ['/users'] } } })
writeModule('alsogreedy', { manifest: { mounts: { public: ['/wiki'] } } })
assert.match(stateOf(alone('greedy', { manifest: { mounts: { admin: ['/users'] } } }, real), 'greedy').reason, /owned by core/)
assert.match(
stateOf(alone('alsogreedy', { manifest: { mounts: { public: ['/wiki'] } } }, real), 'alsogreedy').reason,
/owned by core/,
)
// 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')
assert.equal(stateOf(alone('polite', oneRoute('/widgets', 'player'), real), '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())
const coreTiers = () => {
const tiers = emptyTiers()
tiers.public.use('/posts', express.Router())
tiers.public.use('/', express.Router())
return tiers
}
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/)
assert.equal(stateOf(alone('fine', oneRoute('/widgets'), coreTiers()), 'fine').state, 'registered')
assert.match(stateOf(alone('taken', oneRoute('/posts'), coreTiers()), '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"/)
assert.match(
stateOf(alone('nested', { manifest: { mounts: { public: ['/a/b'] } } }), 'nested').reason,
/bad prefix "\/a\/b"/,
)
assert.match(
stateOf(alone('parameterised', { manifest: { mounts: { public: ['/:id'] } } }), 'parameterised').reason,
/bad prefix "\/:id"/,
)
})
test('registering a prefix that was never declared is rejected', () => {
@@ -314,17 +358,6 @@ test('a schema fragment table must carry the module id as a prefix', () => {
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 })
@@ -356,8 +389,7 @@ test('a fragment may only use the four verbs core\'s own schema.sql uses', () =>
['wiper', 'DELETE FROM wiper_x;', 'DELETE'],
['granter', 'GRANT ALL ON *.* TO app;', 'GRANT'],
]) {
writeModule(id, { schema: sql })
const reason = stateOf(freshLoader(tmpRoot), id).reason
const reason = stateOf(alone(id, { schema: sql }), id).reason
assert.match(reason, new RegExp(`starts with "${verb}"`))
}
})
@@ -386,24 +418,20 @@ test('a CREATE TABLE without IF NOT EXISTS is rejected', () => {
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)
test('fragments() lists only a registered module that has one', () => {
const loader = alone('withdb', { schema: 'CREATE TABLE IF NOT EXISTS withdb_x (id INT);' })
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'))
assert.deepEqual(alone('nodb', oneRoute('/plain')).fragments(), [])
assert.deepEqual(alone('broken', { schema: 'CREATE TABLE IF NOT EXISTS not_mine (id INT);' }).fragments(), [])
})
test('fragments() before load() throws, like every other accessor', () => {
@@ -417,20 +445,20 @@ test('fragments() before load() throws, like every other accessor', () => {
// ── 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)
for (const [id, spec, url, status] of [
['good', oneRoute('/widgets'), '/public/widgets', 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()
['bad', { ...oneRoute('/broken'), server: 'throw new Error("boom")' }, '/public/broken', 404],
]) {
const tiers = emptyTiers()
alone(id, spec, tiers)
const app = await startApp((a) => a.use('/public', tiers.public))
try {
assert.equal((await fetch(`${app.url}${url}`)).status, status, id)
} finally {
await app.close()
}
}
})
@@ -601,24 +629,20 @@ test('a module with no hooks is bootable, and offers nothing to shut down', () =
assert.deepEqual(loader.shutdownHooks(), [])
})
test('shutdown hooks come back in reverse order, and only for started modules', () => {
const hook = 'module.exports = (ctx, api) => api.onShutdown(async () => {})'
writeModule('aaa', { server: hook })
writeModule('bbb', { server: hook })
writeModule('ccc', { server: hook })
const loader = freshLoader(tmpRoot)
test('a shutdown hook comes back only for a started module', () => {
const loader = alone('aaa', { server: 'module.exports = (ctx, api) => api.onShutdown(async () => {})' })
// Nothing has started yet, so there is nothing to tear down.
assert.deepEqual(loader.shutdownHooks(), [])
loader.setState('aaa', 'started')
loader.setState('bbb', 'startup_failed', { stage: 'boot', reason: 'never warmed up' })
loader.setState('ccc', 'started')
assert.deepEqual(loader.shutdownHooks().map((h) => h.id), ['aaa'])
// Reverse registration order (§2.5), and `bbb` is absent: a module whose
// onBoot threw is mid-way through a warm-up it never finished, and handing it
// a half-built world to tear down is worse than not closing cleanly.
assert.deepEqual(loader.shutdownHooks().map((h) => h.id), ['ccc', 'aaa'])
// A module whose onBoot threw is mid-way through a warm-up it never finished,
// and handing it a half-built world to tear down is worse than not closing
// cleanly.
loader.setState('aaa', 'startup_failed', { stage: 'boot', reason: 'never warmed up' })
assert.deepEqual(loader.shutdownHooks(), [])
})
// ── Failure stages ─────────────────────────────────────────────────────────
@@ -636,11 +660,8 @@ test('a failure is recorded against the §4.3 step that produced it', () => {
['f-require', { server: 'throw new Error("boom")' }, 'require'],
['g-register', { server: 'module.exports = (ctx, api) => { throw new Error("nope") }' }, 'register'],
]
for (const [id, spec] of cases) writeModule(id, spec)
const loader = freshLoader(tmpRoot)
for (const [id, , stage] of cases) {
const state = stateOf(loader, id)
for (const [id, spec, stage] of cases) {
const state = stateOf(alone(id, spec), id)
assert.equal(state.state, 'startup_failed', `${id} should have failed`)
assert.equal(state.stage, stage, `${id} should have failed at "${stage}"`)
}
@@ -668,26 +689,30 @@ test('a module that fails AFTER registering leaves nothing in the registries', (
assert.equal(registries.announceLeg('halfway.leg'), null)
})
test('a module colliding with an already-registered name fails alone, unmounted', () => {
test('a module colliding with a name core registered fails alone, unmounted', () => {
// Core registers through the same staging area a module does, and may name
// anything; with one module per site, core is the registrant a module can
// collide with. Staged by hand between the reset and the load, which is the
// order app.js runs registerCore() and modules.load() in.
const tiers = emptyTiers()
writeModule('first', {
manifest: { mounts: { public: ['/first'] } },
server: `module.exports = (ctx, api) => {
api.registerRoutes({ public: { '/first': ctx.express.Router() } })
api.registerNotificationStreams([{ id: 'first.shared', label: 'Shared' }])
}`,
})
writeModule('second', {
manifest: { mounts: { public: ['/second'] } },
server: `module.exports = (ctx, api) => {
api.registerRoutes({ public: { '/second': ctx.express.Router() } })
api.registerNotificationStreams([{ id: 'second.ok', label: 'Ok' }, { id: 'first.shared', label: 'Mine' }])
api.registerNotificationStreams([{ id: 'second.ok', label: 'Ok' }, { id: 'second.shared', label: 'Mine' }])
}`,
})
const loader = freshLoader(tmpRoot, tiers)
process.env.MODULES_DIR = tmpRoot
registries._reset()
const core = registries.stage('core')
core.registerNotificationStreams([{ id: 'second.shared', label: 'Shared' }])
registries.apply(core.staged)
delete require.cache[require.resolve('../src/modules/loader')]
// eslint-disable-next-line global-require
const loader = require('../src/modules/loader')
loader.load(tiers)
assert.equal(stateOf(loader, 'first').state, 'registered')
assert.match(stateOf(loader, 'second').reason, /already registered by "first"/)
assert.match(stateOf(loader, 'second').reason, /already registered by "core"/)
// Not even the claim that did not collide.
assert.equal(registries.isValidStream('second.ok'), false)
// And the loser is not mounted at all. Asked of the live router the way the
@@ -695,7 +720,6 @@ test('a module colliding with an already-registered name fails alone, unmounted'
// produces two (the dispatch guard, then the module's router).
const claims = (prefix) =>
tiers.public.stack.some((l) => l.regexp && !l.regexp.fast_slash && l.match(prefix))
assert.equal(claims('/first'), true)
assert.equal(claims('/second'), false)
})
@@ -756,21 +780,23 @@ test('an entry that escapes the module directory is refused before anything else
test('an entry that is not a .js file, or is missing, is refused', () => {
withChunk('aaa', { entry: 'client/dist/entry.mjs' })
assert.match(stateOf(freshLoader(tmpRoot), 'aaa').reason, /must name a \.js file/)
tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'rg-modules-'))
withChunk('bbb', { entry: 'client/dist/entry.js', write: false })
const loader = freshLoader(tmpRoot)
assert.match(stateOf(loader, 'aaa').reason, /must name a \.js file/)
assert.match(stateOf(loader, 'bbb').reason, /is missing/)
assert.match(stateOf(freshLoader(tmpRoot), 'bbb').reason, /is missing/)
})
test('a malformed client key is a loud failure, not an ignored setting', () => {
writeModule('aaa', { manifest: { client: 'client/dist/entry.js' } })
writeModule('bbb', { manifest: { client: { entry: 'x/e.js', chunks: ['a.js'] } } })
writeModule('ccc', { manifest: { client: {} } })
const loader = freshLoader(tmpRoot)
assert.match(stateOf(loader, 'aaa').reason, /client must be an object/)
assert.match(stateOf(loader, 'bbb').reason, /unknown key "client\.chunks"/)
assert.match(stateOf(loader, 'ccc').reason, /client\.entry must be a path/)
for (const id of ['aaa', 'bbb', 'ccc']) assert.equal(stateOf(loader, id).stage, 'manifest')
for (const [id, client, reason] of [
['aaa', 'client/dist/entry.js', /client must be an object/],
['bbb', { entry: 'x/e.js', chunks: ['a.js'] }, /unknown key "client\.chunks"/],
['ccc', {}, /client\.entry must be a path/],
]) {
const state = stateOf(alone(id, { manifest: { client } }), id)
assert.match(state.reason, reason)
assert.equal(state.stage, 'manifest')
}
})
test('only a STARTED module gets a script tag, though every one keeps its mount', () => {