feat(modules): the three de-entanglement registries, with core as the registrant
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 <noreply@anthropic.com>
This commit is contained in:
@@ -27,8 +27,16 @@ const assert = require('node:assert/strict')
|
||||
const express = require('express')
|
||||
|
||||
const db = require('../src/utils/db')
|
||||
const registries = require('../src/modules/registries')
|
||||
const { startApp } = require('./_helper')
|
||||
|
||||
// Requiring the real admin router declares the `admin.users.detail` extension
|
||||
// slot exactly the way production does (users.router.js, at require time). Doing
|
||||
// it here rather than calling declareSlot by hand matters: one test below builds
|
||||
// the real tier routers, and a hand-declared slot would collide with that
|
||||
// require's own declaration.
|
||||
require('../src/router/v1/admin')
|
||||
|
||||
after(() => db.close())
|
||||
|
||||
let tmpRoot
|
||||
@@ -42,6 +50,11 @@ const emptyTiers = () => ({
|
||||
|
||||
function freshLoader(dir, tiers = emptyTiers()) {
|
||||
process.env.MODULES_DIR = dir
|
||||
// The registries are process-global (there is one core), so hand the process
|
||||
// back between tests. Without this a module's staged registrations from a
|
||||
// previous test would still be committed, and every collision assertion below
|
||||
// would be asserting against the wrong history.
|
||||
registries._reset()
|
||||
delete require.cache[require.resolve('../src/modules/loader')]
|
||||
// eslint-disable-next-line global-require
|
||||
const loader = require('../src/modules/loader')
|
||||
@@ -493,13 +506,10 @@ test('ctx exposes exactly the documented surface, and is frozen', () => {
|
||||
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.
|
||||
test('the register calls PR 5 owns throw rather than silently accepting', () => {
|
||||
// An accepting no-op would let a module believe it had registered 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],
|
||||
]) {
|
||||
@@ -511,3 +521,56 @@ test('the register calls PR 4 and PR 5 own throw rather than silently accepting'
|
||||
)
|
||||
}
|
||||
})
|
||||
|
||||
// ── Staged registrations are committed only for a module that survives ─────
|
||||
|
||||
test('a module that fails AFTER registering leaves nothing in the registries', () => {
|
||||
// The registry-side twin of the second-pass mount rule. register() runs before
|
||||
// checkDeclared, so a module can stage a stream catalog and then be rejected —
|
||||
// and a half-registered catalog is worse than a missing one, because it is a
|
||||
// subscribable stream nothing will ever publish to.
|
||||
writeModule('halfway', {
|
||||
manifest: { mounts: { public: ['/declared'] } },
|
||||
server: `module.exports = (ctx, api) => {
|
||||
api.registerNotificationStreams([{ id: 'halfway.thing', label: 'Thing' }])
|
||||
api.registerAnnounceLeg({ leg: 'halfway.leg', label: 'L', dispatch: async () => ({}), classify: () => ({}) })
|
||||
// declared /declared and never registered it → rejected by checkDeclared
|
||||
}`,
|
||||
})
|
||||
const loader = freshLoader(tmpRoot)
|
||||
|
||||
assert.match(stateOf(loader, 'halfway').reason, /declared public\/declared but never registered it/)
|
||||
assert.equal(registries.isValidStream('halfway.thing'), false)
|
||||
assert.equal(registries.announceLeg('halfway.leg'), null)
|
||||
})
|
||||
|
||||
test('a module colliding with an already-registered name fails alone, unmounted', () => {
|
||||
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' }])
|
||||
}`,
|
||||
})
|
||||
const loader = freshLoader(tmpRoot, tiers)
|
||||
|
||||
assert.equal(stateOf(loader, 'first').state, 'registered')
|
||||
assert.match(stateOf(loader, 'second').reason, /already registered by "first"/)
|
||||
// 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
|
||||
// prefix-ownership check asks it, rather than by counting layers — one mount
|
||||
// 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)
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user