From f40dd765c31fa152e167e69a11223f2689cc467b Mon Sep 17 00:00:00 2001 From: wtclaude Date: Thu, 22 Oct 2026 00:38:34 -0500 Subject: [PATCH] fix(core): the last fixes' core half (W1 W2 W5 W19) 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 Claude-Session: https://claude.ai/code/session_01E14m6SuuY6i1vASFeGDBeY --- client/src/routes/admin/views/EventEditor.jsx | 16 +- client/src/routes/admin/views/EventRun.jsx | 8 +- server/db/schema.sql | 12 + server/src/events/cleanup.js | 74 +++-- server/src/events/spec.js | 28 ++ .../src/model/events/eventDefinitions.db.js | 13 + .../model/events/eventDefinitions.model.js | 37 ++- server/src/modules/loader.js | 61 +++- .../src/router/v1/admin/events.controller.js | 12 +- server/src/router/v1/admin/events.router.js | 8 +- server/swagger/swagger-output.json | 14 +- server/test/eventCleanup.test.js | 65 ++++- server/test/eventModuleContract.test.js | 10 + server/test/eventSpec.test.js | 17 ++ server/test/eventsAdmin.test.js | 106 +++++++ server/test/moduleLifecycle.test.js | 84 +++--- server/test/moduleLoader.test.js | 260 ++++++++++-------- server/test/moduleSchema.test.js | 17 +- 18 files changed, 632 insertions(+), 210 deletions(-) diff --git a/client/src/routes/admin/views/EventEditor.jsx b/client/src/routes/admin/views/EventEditor.jsx index 3a2b2b9..a55463e 100644 --- a/client/src/routes/admin/views/EventEditor.jsx +++ b/client/src/routes/admin/views/EventEditor.jsx @@ -1497,11 +1497,16 @@ export default function EventEditor() { setEvent(result.event) setVersions(await api.admin.listEventVersions(id).then((r) => r.versions || [])) // The re-pin count is said out loud, because an editor who does not know - // their fix reached next Friday finds out on Friday. + // their fix reached next Friday finds out on Friday. So is a dry run that + // carried over from the draft (W5): it is why no second one is asked for. setNotice( - result.repinned - ? `Published as v${result.version}. ${result.repinned} scheduled occurrence${result.repinned === 1 ? '' : 's'} moved to it.` - : `Published as v${result.version}.`, + [ + `Published as v${result.version}.`, + result.verified ? 'The dry run that passed on this draft counts for it.' : '', + result.repinned + ? `${result.repinned} scheduled occurrence${result.repinned === 1 ? '' : 's'} moved to it.` + : '', + ].filter(Boolean).join(' '), ) } catch (err) { setProblems(err.body?.errors || [err.message]) @@ -1674,6 +1679,7 @@ export default function EventEditor() { report that did not say would be read as the other one. */} {report.target === 'version' ? `published v${report.version}` : 'the working draft'} {report.recorded && ' · recorded, so scheduled occurrences may now start'} + {report.draftRecorded && ' · kept, so publishing this draft unchanged makes a verified version'}

@@ -2019,7 +2025,7 @@ export default function EventEditor() {

- The key is what the run console groups by and what “phase 3 has not started” names, so it + The key is what the run console groups by and what “Phase p3: steps done, waiting to advance” names, so it cannot change once runs exist.

diff --git a/client/src/routes/admin/views/EventRun.jsx b/client/src/routes/admin/views/EventRun.jsx index 1833d7c..d8de1a9 100644 --- a/client/src/routes/admin/views/EventRun.jsx +++ b/client/src/routes/admin/views/EventRun.jsx @@ -132,7 +132,9 @@ function GateRow({ gate, current }) {
Phase {gate.phase} - {current && !gate.satisfied ? ' has not started' : ''} + {/* W2: the phase is running and its steps are done; it is the advance + that waits, so "has not started" was the wrong sentence. */} + {current && !gate.satisfied ? ': steps done, waiting to advance' : ''} {gate.satisfied && ` — released ${gate.satisfiedBy === 'forced' ? 'by hand' : `on its ${gate.satisfiedBy === 'elapsed' ? 'deadline' : 'condition'}`}`} {gate.stalled && ' — STALLED'}
@@ -389,7 +391,7 @@ export default function EventRun() {

)} - {/* ── Why this phase has not started (Phase 5) ── + {/* ── Why this phase has not advanced (Phase 5) ── Above the step list for the same reason the parked cue is: a phase waiting on a condition is `running` and looks completely healthy, and the one screen an operator opens to find out why nothing is happening @@ -400,7 +402,7 @@ export default function EventRun() { style={{ padding: 14, marginBottom: 14, borderLeft: `3px solid ${waiting ? (waiting.stalled ? '#d98b84' : '#d9c184') : 'var(--rule)'}` }} >

- {waiting ? 'Why this phase has not started' : 'Phase advance conditions'} + {waiting ? 'Why this phase has not advanced' : 'Phase advance conditions'}

{waiting ? ( diff --git a/server/db/schema.sql b/server/db/schema.sql index d5a281f..7dd569e 100644 --- a/server/db/schema.sql +++ b/server/db/schema.sql @@ -2608,6 +2608,18 @@ ALTER TABLE event_run_resources MODIFY COLUMN status ALTER TABLE event_versions ADD COLUMN IF NOT EXISTS verified_at DATETIME NULL; ALTER TABLE event_versions ADD COLUMN IF NOT EXISTS verified_by INT NULL; +-- A dry run that passed on the DRAFT, and the fingerprint of the spec it passed +-- on (W5, Rust PLAN_LAST_FIXES). A draft changes under the author's hands, so a +-- pass on it is not a property of any version -- but publish compares this +-- fingerprint with the spec it is about to freeze, and when they are equal the +-- new version is stamped verified with this pass's reviewer and time. Without it +-- an author who dry-ran and then published found v1 "not dry run", and its +-- scheduled and trigger starts held, until they ran the same check again. An +-- edit after the dry run changes the fingerprint, so nothing stale carries. +ALTER TABLE event_definitions ADD COLUMN IF NOT EXISTS draft_verified_at DATETIME NULL; +ALTER TABLE event_definitions ADD COLUMN IF NOT EXISTS draft_verified_by INT NULL; +ALTER TABLE event_definitions ADD COLUMN IF NOT EXISTS draft_verified_hash CHAR(64) NULL; + -- Whether this definition appears on the PUBLIC calendar (Phase 14a). -- -- **Not a second answer to the question `state` answers**, which is the trap the diff --git a/server/src/events/cleanup.js b/server/src/events/cleanup.js index 97951b6..40879f9 100644 --- a/server/src/events/cleanup.js +++ b/server/src/events/cleanup.js @@ -357,13 +357,16 @@ async function sweep() { * Reverting it would be core recording that it put something back when what * actually happened is that the thing vanished while nobody was looking, and the * two are different sentences to the operator reading the console afterwards. + * One gone after its `until` is neither: the game ended it on time, so it is + * `expired`, the line `expired()` would have written (W1). * * A module with no `reconcile()` on the action is not broken: core keeps * believing its own ledger, which is precisely the behaviour before this phase. */ -async function reconcileModule(owner) { +async function reconcileModule(owner, now = new Date()) { const rows = await resourcesDb.liveForModule(owner) - const summary = { asked: 0, inForce: 0, orphaned: 0, unanswered: 0 } + const summary = { asked: 0, inForce: 0, orphaned: 0, expired: 0, unanswered: 0 } + const settled = new Set() if (!rows.length) return summary const byStep = new Map() @@ -411,6 +414,19 @@ async function reconcileModule(owner) { summary.inForce += 1 continue } + // **Gone after its deadline is the deadline working** (W1, Rust + // PLAN_LAST_FIXES). A resource that carries `until` is one the game ends + // by itself, and on a reconnect the module's reconcile can run before the + // game's backlog says so: the run log read `resource.orphaned` five seconds + // before `resource.expired`. Past its `until`, gone is expired. Before it, + // gone is still nobody-was-looking, which is what orphaned means. + if (row.lease_until && new Date(row.lease_until).getTime() <= now.getTime()) { + if (await settleExpired(owner, row)) { + summary.expired += 1 + settled.add(row.run_id) + } + continue + } await resourcesDb.markOrphaned(row.id, 'the module reports this is no longer in force') summary.orphaned += 1 await logDb.write({ @@ -420,9 +436,42 @@ async function reconcileModule(owner) { }) } } + await closeSettledRuns(settled) return summary } +/** + * One row the game ended at its deadline: `expired`, and its log line. The same + * line whichever way core learned it, from the module's `expired()` or from a + * reconcile that found it gone past its `until`, so the console reads one way. + */ +async function settleExpired(owner, row) { + if (!(await resourcesDb.markExpired(row.id, 'the game ended it at its deadline'))) return false + await logDb.write({ + runId: row.run_id, + kind: 'resource.expired', + detail: { module: owner, resource: `${row.kind}:${row.ref}` }, + }) + return true +} + +/** + * A finished run whose last unresolved row just expired has nothing left to + * clean up. The sweep would settle a `pending` one by itself, but not an + * `incomplete` one — that status takes a run out of the sweep's scan, and it + * would go on saying "not finished" about a ledger that is. A run still in + * flight is left alone: its own terminal path computes the status. + */ +async function closeSettledRuns(runIds) { + for (const runId of runIds) { + const run = await runsDb.getById(runId) + if (!run || !runsDb.TERMINAL.includes(run.status)) continue + if ((await resourcesDb.unresolvedCount(runId)) === 0) { + await runsDb.setCleanupStatus(runId, 'complete', ['pending', 'incomplete']) + } + } +} + /** Ask every module that owns a live row. Core's own boot-time sweep. */ async function reconcileAll() { const owners = await resourcesDb.modulesWithLiveRows() @@ -469,28 +518,11 @@ async function expireResource(owner, { kind, ref } = {}) { let expired = 0 for (const row of rows) { - if (!(await resourcesDb.markExpired(row.id, 'the game ended it at its deadline'))) continue + if (!(await settleExpired(owner, row))) continue expired += 1 runs.add(row.run_id) - await logDb.write({ - runId: row.run_id, - kind: 'resource.expired', - detail: { module: owner, resource: `${row.kind}:${row.ref}` }, - }) - } - - // A finished run whose last unresolved row this was has nothing left to clean - // up. The sweep would settle a `pending` one by itself, but not an - // `incomplete` one — that status takes a run out of the sweep's scan, and it - // would go on saying "not finished" about a ledger that is. A run still in - // flight is left alone: its own terminal path computes the status. - for (const runId of runs) { - const run = await runsDb.getById(runId) - if (!run || !runsDb.TERMINAL.includes(run.status)) continue - if ((await resourcesDb.unresolvedCount(runId)) === 0) { - await runsDb.setCleanupStatus(runId, 'complete', ['pending', 'incomplete']) - } } + await closeSettledRuns(runs) return { expired } } diff --git a/server/src/events/spec.js b/server/src/events/spec.js index d39b4bd..4fd2f27 100644 --- a/server/src/events/spec.js +++ b/server/src/events/spec.js @@ -29,6 +29,8 @@ // DST gap shifts it, it is never rejected), so a schedule that validates here // computes there. +const crypto = require('crypto') + const registries = require('../modules/registries') // For `priceOf` and `undeclaredDimensions` only — the save-time half of §F's // fail-closed budget rule (Phase 7). Nothing here reaches the database: @@ -730,6 +732,31 @@ function publishable(spec) { return dormant.length ? { ok: false, dormant: [...new Set(dormant)] } : { ok: true, dormant: [] } } +/** JSON with every object's keys sorted, so equal specs serialise equally. */ +function canonical(value) { + if (Array.isArray(value)) return `[${value.map(canonical).join(',')}]` + if (value && typeof value === 'object') { + const keys = Object.keys(value).filter((k) => value[k] !== undefined).sort() + return `{${keys.map((k) => `${JSON.stringify(k)}:${canonical(value[k])}`).join(',')}}` + } + return JSON.stringify(value) ?? 'null' +} + +/** + * A spec's fingerprint: the sha256 of its canonical JSON (W5, Rust + * PLAN_LAST_FIXES). + * + * What lets a dry run that passed on a draft count for the version published + * from it. The draft is fingerprinted when the dry run passes and the version + * when it is published, both from `validate`'s output, so an edit in between is + * a different fingerprint and nothing carries over. Key order is not part of it: + * a spec that round-trips through the database with its keys reordered is the + * same plan. + */ +function fingerprint(spec) { + return crypto.createHash('sha256').update(canonical(spec)).digest('hex') +} + /** An empty, valid spec — what a newly created draft carries. */ const emptySpec = () => ({ schedule: { kind: 'manual' }, @@ -742,6 +769,7 @@ module.exports = { // 1.13.0): eventDefinitions.model `buildFromTemplate`. checkParams, publishable, + fingerprint, actionIdsIn, emptySpec, defaultOnFailure, diff --git a/server/src/model/events/eventDefinitions.db.js b/server/src/model/events/eventDefinitions.db.js index b906f0e..78b868f 100644 --- a/server/src/model/events/eventDefinitions.db.js +++ b/server/src/model/events/eventDefinitions.db.js @@ -126,6 +126,18 @@ const markReady = (id, versionId, userId) => [versionId, userId, id], ) +/** + * Record a dry run that passed on the draft, with the fingerprint of the spec it + * passed on (W5). Publish reads it back through `getById`'s `d.*`. + */ +const markDraftVerified = (id, { at = new Date(), userId = null, hash }) => + query( + `UPDATE event_definitions + SET draft_verified_at = ?, draft_verified_by = ?, draft_verified_hash = ? + WHERE id = ?`, + [at, userId, hash, id], + ) + /** * Every definition the runner should expand a recurrence for (Phase 4). * @@ -234,5 +246,6 @@ module.exports = { insert, update, markReady, + markDraftVerified, archive, } diff --git a/server/src/model/events/eventDefinitions.model.js b/server/src/model/events/eventDefinitions.model.js index e10c6b9..d968312 100644 --- a/server/src/model/events/eventDefinitions.model.js +++ b/server/src/model/events/eventDefinitions.model.js @@ -249,6 +249,16 @@ async function publish(id, userId) { const versionId = await versionsDb.insert(id, version, checked.spec, userId) await db.markReady(id, versionId, userId) + // A dry run that passed on this exact draft counts for the version made from + // it (W5), stamped with that pass's reviewer and time rather than the + // publisher's and now: "who last looked at this before it ran unattended" is + // the person who ran the check. Any edit since is a different fingerprint. + const verified = Boolean( + existing.draft_verified_hash && + existing.draft_verified_hash === spec.fingerprint(checked.spec) && + (await versionsDb.markVerified(versionId, existing.draft_verified_by || null, existing.draft_verified_at || new Date())), + ) + // Occurrences already materialised ahead of their instant move to the new // version; ones that have begun do not (org lead, 2026-09-02). Logged per run // rather than only counted, because "which version did this run actually use" @@ -270,9 +280,18 @@ async function publish(id, userId) { by: userId, }, }) + // The held occurrence is now going to start, and the line saying why + // belongs on it, as it does when the version is dry-run by hand. + if (verified) { + await logDb.write({ + runId: run.id, + kind: 'version.verified', + detail: { versionId, version, by: existing.draft_verified_by || null, fromDraft: true }, + }) + } } - return { ok: true, versionId, version, repinned, definition: await db.getById(id) } + return { ok: true, versionId, version, repinned, verified, definition: await db.getById(id) } } /** @@ -373,6 +392,19 @@ async function verify(id, user) { const report = await verifier.verifySpec(target, { user }) + // A pass on the draft is kept with the fingerprint of the spec it passed on, + // for publish to compare (W5). Fingerprinted from `validate`'s output, as + // publish fingerprints the spec it freezes; a draft that no longer validates + // could not be published anyway, so there is nothing to keep. + let draftRecorded = false + if (!version && report.ok) { + const checked = spec.validate(existing.spec, { knownActionIds: spec.actionIdsIn(existing.spec) }) + if (checked.ok) { + await db.markDraftVerified(id, { userId: user?.id || null, hash: spec.fingerprint(checked.spec) }) + draftRecorded = true + } + } + if (version && report.ok) { await versionsDb.markVerified(version.id, user?.id || null) // Written to every run already pinned to this version, because that is where @@ -397,6 +429,9 @@ async function verify(id, user) { versionId: version?.id || null, version: version?.version || null, recorded: Boolean(version && report.ok), + // A pass on the draft, kept so that publishing this spec unchanged makes a + // verified version (W5). + draftRecorded, } } diff --git a/server/src/modules/loader.js b/server/src/modules/loader.js index 4a82eca..4355a4b 100644 --- a/server/src/modules/loader.js +++ b/server/src/modules/loader.js @@ -813,10 +813,26 @@ function load(tierRouters) { return // no modules directory is the normal case for a bare core } + // One module per site, at boot as at install (`install.js`, org lead + // 2026-09-23; W19 in Rust PLAN_LAST_FIXES, 2026-10-22). Install already + // refuses a second module, so two on the volume means someone copied one in by + // hand. The contract's singletons assume one: Module-uo and Module-Rust both + // fill `admin.users.detail`, and the second used to fail at register with a + // slot collision that named neither the rule nor the fix. Now the first in + // scan order is the site's module, whether or not it then loads, and every + // other is refused before a line of its code is required. + let holder = null + for (const id of entries) { const dir = path.join(MODULES_DIR, id) if (!fs.existsSync(path.join(dir, 'module.json'))) continue + if (holder) { + refuseSecond(id, dir, holder) + continue + } + holder = id + const record = { id, dir, @@ -886,8 +902,10 @@ function load(tierRouters) { for (const record of modules.values()) { if (record.state !== 'registered') continue try { - // Commit what this module staged. Collisions with core or with an earlier - // module surface here, in scan order, and cost only this module. + // Commit what this module staged. Collisions with core surface here and + // cost only this module. (An earlier module is no longer possible — one + // module per site, above — and the module-against-module checks stay as + // the second line behind that rule.) registries.apply(record.staged.staged) } catch (err) { record.state = 'startup_failed' @@ -902,6 +920,45 @@ function load(tierRouters) { } } +/** + * Record a module refused because another already holds the site (W19). + * + * `startup_failed` at the `manifest` stage, like any module that never reached + * register: no routes, no nav, no tables, and Admin → Modules says why. The + * name and version are read for that panel and nothing else; a module.json that does not + * parse is still refused for the same reason, since the reason is not its + * contents. + */ +function refuseSecond(id, dir, holder) { + const manifest = { id, version: 'unknown' } + try { + const raw = JSON.parse(fs.readFileSync(path.join(dir, 'module.json'), 'utf8')) + if (raw && typeof raw.version === 'string') manifest.version = raw.version + if (raw && typeof raw.name === 'string') manifest.name = raw.name + } catch { + // the panel shows "unknown"; the refusal stands either way + } + const reason = + `this site already runs the module "${holder}", and a site runs one module. ` + + `Remove "${id}" or "${holder}" from the modules directory; the one that sorts first by name is the one that starts.` + modules.set(id, { + id, + dir, + manifest, + routes: { public: new Map(), admin: new Map(), player: new Map() }, + staged: null, + tables: new Set(), + called: new Set(), + hooks: { onBoot: null, onShutdown: null, onIdentityLinking: null, onIdentityLinked: null }, + client: null, + ctx: null, + state: 'startup_failed', + stage: 'manifest', + reason, + }) + log.error(`module "${id}" was not started — continuing without it`, { stage: 'manifest', reason }) +} + /** * Mount one module's routers onto the tier routers, behind the dispatch guard. * diff --git a/server/src/router/v1/admin/events.controller.js b/server/src/router/v1/admin/events.controller.js index 232fdae..1e16af5 100644 --- a/server/src/router/v1/admin/events.controller.js +++ b/server/src/router/v1/admin/events.controller.js @@ -445,7 +445,13 @@ exports.publish = async (req, res) => { await activity.log({ req, action: 'event.definition.published', - detail: { id, version: result.version, versionId: result.versionId, repinned: result.repinned }, + detail: { + id, + version: result.version, + versionId: result.versionId, + repinned: result.repinned, + verified: result.verified, + }, }) return res.json({ event: await shapeEditable(result.definition), @@ -455,6 +461,9 @@ exports.publish = async (req, res) => { // screen says so, because "my fix did not reach next Friday" is otherwise // found out on Friday. repinned: result.repinned, + // Whether a dry run that passed on this exact draft made the version + // verified at birth (W5), so the screen can say it needs no second one. + verified: result.verified, }) } @@ -492,6 +501,7 @@ exports.verify = async (req, res) => { versionId: result.versionId, version: result.version, recorded: result.recorded, + draftRecorded: result.draftRecorded, report: result.report, }) } diff --git a/server/src/router/v1/admin/events.router.js b/server/src/router/v1/admin/events.router.js index e1320e8..5aff4c8 100644 --- a/server/src/router/v1/admin/events.router.js +++ b/server/src/router/v1/admin/events.router.js @@ -475,9 +475,9 @@ eventsRouter.post( '/:id/verify', // #swagger.tags = ['Admin · Events'] // #swagger.summary = 'Dry run: dispatch every step with verify true, change nothing, and report the cost against the caps' - // #swagger.description = 'EVENTS.md I. admin AND editor rather than admin, deliberately: a dry run dispatches nothing, and the author who wrote the definition is exactly who should be able to price it before asking an admin to publish it. What is verified follows the state -- a ready definition is checked against its PUBLISHED version, which is the only thing that ever actually runs, and a draft against the working spec the author is still holding; `target` says which. A pass against a version is RECORDED on it, and that is EVENTS.md K last bound: a scheduled occurrence of a version that has never been verified is held rather than started unattended. Findings come back with a 200 -- the request succeeded, the plan has problems -- and the whole-plan cost check is the one thing no other path makes: three steps each spawning 15 under a cap of 30 pass every individual check and breach it on the third, at two in the morning, with the world half-changed.' + // #swagger.description = 'EVENTS.md I. admin AND editor rather than admin, deliberately: a dry run dispatches nothing, and the author who wrote the definition is exactly who should be able to price it before asking an admin to publish it. What is verified follows the state -- a ready definition is checked against its PUBLISHED version, which is the only thing that ever actually runs, and a draft against the working spec the author is still holding; `target` says which. A pass against a version is RECORDED on it, and that is EVENTS.md K last bound: a scheduled occurrence of a version that has never been verified is held rather than started unattended. A pass against a draft is kept with the fingerprint of the spec it passed on (`draftRecorded`), and publishing that spec unchanged makes a version that is verified from the start; an edit in between changes the fingerprint, and nothing carries over. Findings come back with a 200 -- the request succeeded, the plan has problems -- and the whole-plan cost check is the one thing no other path makes: three steps each spawning 15 under a cap of 30 pass every individual check and breach it on the third, at two in the morning, with the world half-changed.' // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] - /* #swagger.responses[200] = { description: 'The report: findings per step, and the total cost per budget dimension', content: { "application/json": { schema: { type: "object", properties: { target: { type: "string", example: "version" }, versionId: { type: "integer" }, version: { type: "integer" }, recorded: { type: "boolean" }, report: { type: "object", properties: { ok: { type: "boolean" }, steps: { type: "integer" }, findings: { type: "array", items: { type: "object", additionalProperties: true } }, cost: { type: "array", items: { type: "object", additionalProperties: true } } } } } } } } } */ + /* #swagger.responses[200] = { description: 'The report: findings per step, and the total cost per budget dimension', content: { "application/json": { schema: { type: "object", properties: { target: { type: "string", example: "version" }, versionId: { type: "integer" }, version: { type: "integer" }, recorded: { type: "boolean" }, draftRecorded: { type: "boolean" }, report: { type: "object", properties: { ok: { type: "boolean" }, steps: { type: "integer" }, findings: { type: "array", items: { type: "object", additionalProperties: true } }, cost: { type: "array", items: { type: "object", additionalProperties: true } } } } } } } } } */ /* #swagger.responses[404] = { description: 'No such definition', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ /* #swagger.responses[409] = { description: 'The definition is archived, or has no phases to verify', content: { "application/json": { schema: { type: "object", properties: { errors: { type: "array", items: { type: "string" } } } } } } } */ /* #swagger.responses[403] = { description: 'Not an admin or editor', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ @@ -489,9 +489,9 @@ eventsRouter.post( '/:id/publish', // #swagger.tags = ['Admin · Events'] // #swagger.summary = 'Snapshot the working spec into an immutable version and mark the definition ready' - // #swagger.description = 'Admin only, deliberately, and not the same gate as the live run controls: publishing commits a definition that a schedule will later start unattended. The spec is re-validated against the registries as they stand right now rather than trusted from the save that wrote it, so a module uninstalled in between blocks the publish instead of producing a run that fails at dispatch. Publishing also RE-PINS every occurrence of this definition that is still scheduled and has not started, and `repinned` says how many moved: occurrences are materialised a fortnight ahead, so without this an edit would reach none of the runs already on the calendar. A run that has begun keeps the version it pinned.' + // #swagger.description = 'Admin only, deliberately, and not the same gate as the live run controls: publishing commits a definition that a schedule will later start unattended. The spec is re-validated against the registries as they stand right now rather than trusted from the save that wrote it, so a module uninstalled in between blocks the publish instead of producing a run that fails at dispatch. Publishing also RE-PINS every occurrence of this definition that is still scheduled and has not started, and `repinned` says how many moved: occurrences are materialised a fortnight ahead, so without this an edit would reach none of the runs already on the calendar. A run that has begun keeps the version it pinned. `verified` is true when a dry run that passed on this exact draft made the new version verified, with that dry run reviewer and time, so it needs no second dry run before it starts unattended.' // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] - /* #swagger.responses[200] = { description: 'The definition, now ready, the version that was cut, and how many scheduled occurrences moved to it', content: { "application/json": { schema: { type: "object", properties: { event: { type: "object", additionalProperties: true }, version: { type: "integer" }, versionId: { type: "integer" }, repinned: { type: "integer" } } } } } } */ + /* #swagger.responses[200] = { description: 'The definition, now ready, the version that was cut, and how many scheduled occurrences moved to it', content: { "application/json": { schema: { type: "object", properties: { event: { type: "object", additionalProperties: true }, version: { type: "integer" }, versionId: { type: "integer" }, repinned: { type: "integer" }, verified: { type: "boolean" } } } } } } */ /* #swagger.responses[400] = { description: 'The spec is invalid, or no phase has any steps', content: { "application/json": { schema: { type: "object", properties: { errors: { type: "array", items: { type: "string" } } } } } } } */ /* #swagger.responses[409] = { description: 'A step names an action no module registers, or the definition is archived', content: { "application/json": { schema: { type: "object", properties: { errors: { type: "array", items: { type: "string" } } } } } } } */ /* #swagger.responses[403] = { description: 'Not an admin', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ diff --git a/server/swagger/swagger-output.json b/server/swagger/swagger-output.json index 2676060..db7d9cb 100644 --- a/server/swagger/swagger-output.json +++ b/server/swagger/swagger-output.json @@ -352,7 +352,7 @@ "Admin · Auth Providers" ], "summary": "Update an SSO provider (admin only)", - "description": "", + "description": "Built-ins (google, discord, steam) take only enabled + credentials. Steam has no client id: its secret is a Steam Web API key, and enabling Steam without one stored or given is refused with 400 (STEAM_SIGNIN.md D394).", "parameters": [ { "name": "id", @@ -376,7 +376,7 @@ } }, "400": { - "description": "Validation error", + "description": "Validation error, or Steam enabled without a Steam Web API key", "content": { "application/json": { "schema": { @@ -5962,7 +5962,7 @@ "Admin · Events" ], "summary": "Snapshot the working spec into an immutable version and mark the definition ready", - "description": "Admin only, deliberately, and not the same gate as the live run controls: publishing commits a definition that a schedule will later start unattended. The spec is re-validated against the registries as they stand right now rather than trusted from the save that wrote it, so a module uninstalled in between blocks the publish instead of producing a run that fails at dispatch. Publishing also RE-PINS every occurrence of this definition that is still scheduled and has not started, and `repinned` says how many moved: occurrences are materialised a fortnight ahead, so without this an edit would reach none of the runs already on the calendar. A run that has begun keeps the version it pinned.", + "description": "Admin only, deliberately, and not the same gate as the live run controls: publishing commits a definition that a schedule will later start unattended. The spec is re-validated against the registries as they stand right now rather than trusted from the save that wrote it, so a module uninstalled in between blocks the publish instead of producing a run that fails at dispatch. Publishing also RE-PINS every occurrence of this definition that is still scheduled and has not started, and `repinned` says how many moved: occurrences are materialised a fortnight ahead, so without this an edit would reach none of the runs already on the calendar. A run that has begun keeps the version it pinned. `verified` is true when a dry run that passed on this exact draft made the new version verified, with that dry run reviewer and time, so it needs no second dry run before it starts unattended.", "parameters": [ { "name": "id", @@ -5993,6 +5993,9 @@ }, "repinned": { "type": "integer" + }, + "verified": { + "type": "boolean" } } } @@ -6188,7 +6191,7 @@ "Admin · Events" ], "summary": "Dry run: dispatch every step with verify true, change nothing, and report the cost against the caps", - "description": "EVENTS.md I. admin AND editor rather than admin, deliberately: a dry run dispatches nothing, and the author who wrote the definition is exactly who should be able to price it before asking an admin to publish it. What is verified follows the state -- a ready definition is checked against its PUBLISHED version, which is the only thing that ever actually runs, and a draft against the working spec the author is still holding; `target` says which. A pass against a version is RECORDED on it, and that is EVENTS.md K last bound: a scheduled occurrence of a version that has never been verified is held rather than started unattended. Findings come back with a 200 -- the request succeeded, the plan has problems -- and the whole-plan cost check is the one thing no other path makes: three steps each spawning 15 under a cap of 30 pass every individual check and breach it on the third, at two in the morning, with the world half-changed.", + "description": "EVENTS.md I. admin AND editor rather than admin, deliberately: a dry run dispatches nothing, and the author who wrote the definition is exactly who should be able to price it before asking an admin to publish it. What is verified follows the state -- a ready definition is checked against its PUBLISHED version, which is the only thing that ever actually runs, and a draft against the working spec the author is still holding; `target` says which. A pass against a version is RECORDED on it, and that is EVENTS.md K last bound: a scheduled occurrence of a version that has never been verified is held rather than started unattended. A pass against a draft is kept with the fingerprint of the spec it passed on (`draftRecorded`), and publishing that spec unchanged makes a version that is verified from the start; an edit in between changes the fingerprint, and nothing carries over. Findings come back with a 200 -- the request succeeded, the plan has problems -- and the whole-plan cost check is the one thing no other path makes: three steps each spawning 15 under a cap of 30 pass every individual check and breach it on the third, at two in the morning, with the world half-changed.", "parameters": [ { "name": "id", @@ -6220,6 +6223,9 @@ "recorded": { "type": "boolean" }, + "draftRecorded": { + "type": "boolean" + }, "report": { "type": "object", "properties": { diff --git a/server/test/eventCleanup.test.js b/server/test/eventCleanup.test.js index abf6eed..30e4188 100644 --- a/server/test/eventCleanup.test.js +++ b/server/test/eventCleanup.test.js @@ -478,7 +478,7 @@ test('a resource the module no longer has becomes orphaned, never reverted', asy addResource({ ref: '0x2' }) const summary = await cleanup.reconcileModule('demo') - assert.deepEqual(summary, { asked: 2, inForce: 1, orphaned: 1, unanswered: 0 }) + assert.deepEqual(summary, { asked: 2, inForce: 1, orphaned: 1, expired: 0, unanswered: 0 }) assert.equal([...store.rows.values()].find((r) => r.ref === '0x2').status, 'orphaned') assert.equal([...store.rows.values()].find((r) => r.ref === '0x1').status, 'confirmed') assert.ok(store.log.some((e) => e.kind === 'resource.orphaned')) @@ -510,7 +510,7 @@ test('a module with no reconcile is not broken; core keeps believing its ledger' addStep(1, 'demo.spawn') addResource() const summary = await cleanup.reconcileModule('demo') - assert.deepEqual(summary, { asked: 0, inForce: 0, orphaned: 0, unanswered: 1 }) + assert.deepEqual(summary, { asked: 0, inForce: 0, orphaned: 0, expired: 0, unanswered: 1 }) assert.equal([...store.rows.values()][0].status, 'confirmed') }) @@ -532,7 +532,7 @@ test('placeholders are not asked about, because there is nothing to ask yet', as addStep(1, 'demo.spawn') addResource({ kind: resourcesDb.STEP_KIND, ref: 'a'.repeat(40), status: 'pending' }) const summary = await cleanup.reconcileModule('demo') - assert.deepEqual(summary, { asked: 0, inForce: 0, orphaned: 0, unanswered: 0 }) + assert.deepEqual(summary, { asked: 0, inForce: 0, orphaned: 0, expired: 0, unanswered: 0 }) assert.equal([...store.rows.values()][0].status, 'pending') }) @@ -556,7 +556,7 @@ test('a lease row had no reconcile path at all until core.lease grew one', async addResource({ kind: 'override', ref: 'demo.rate', payload: { baseline: 1, applied: 3 } }) const summary = await cleanup.reconcileModule('demo') - assert.deepEqual(summary, { asked: 1, inForce: 0, orphaned: 1, unanswered: 0 }) + assert.deepEqual(summary, { asked: 1, inForce: 0, orphaned: 1, expired: 0, unanswered: 0 }) assert.equal([...store.rows.values()][0].status, 'orphaned') }) @@ -566,7 +566,7 @@ test('a lease the shard still has a record of stays put', async () => { addResource({ kind: 'override', ref: 'demo.rate', payload: { baseline: 1, applied: 3 } }) const summary = await cleanup.reconcileModule('demo') - assert.deepEqual(summary, { asked: 1, inForce: 1, orphaned: 0, unanswered: 0 }) + assert.deepEqual(summary, { asked: 1, inForce: 1, orphaned: 0, expired: 0, unanswered: 0 }) assert.equal([...store.rows.values()][0].status, 'confirmed') }) @@ -705,6 +705,61 @@ test('an expiry nobody ledgered, or a malformed one, is not an error', async () assert.equal(store.log.length, 0) }) +// W1 (Rust PLAN_LAST_FIXES): on a reconnect the module's reconcile can run +// before the game's backlog reports an expiry. A row past its `until` that the +// game no longer has is the deadline working, so it is filed expired at once +// rather than orphaned and then expired five seconds later. + +test('a reconcile files a row gone past its until as expired, not orphaned', async () => { + stubExpiry() + registerAction({ reconcile: async () => ({ ok: true, inForce: [] }) }) + addStep(1, 'demo.spawn') + const now = new Date('2026-10-22T14:06:35Z') + addResource({ kind: 'zone', ref: 'past', lease_until: new Date('2026-10-22T14:06:00Z') }) + addResource({ kind: 'zone', ref: 'future', lease_until: new Date('2026-10-22T14:30:00Z') }) + addResource({ kind: 'zone', ref: 'none' }) + + const summary = await cleanup.reconcileModule('demo', now) + assert.deepEqual(summary, { asked: 3, inForce: 0, orphaned: 2, expired: 1, unanswered: 0 }) + const status = (ref) => [...store.rows.values()].find((r) => r.ref === ref).status + assert.equal(status('past'), 'expired') + // Before its deadline, or with none, gone is still nobody-was-looking. + assert.equal(status('future'), 'orphaned') + assert.equal(status('none'), 'orphaned') + assert.deepEqual( + store.log.filter((e) => e.detail.resource === 'zone:past').map((e) => e.kind), + ['resource.expired'], + ) + + // The game's own word arriving afterwards finds nothing left to close. + assert.deepEqual(await cleanup.expireResource('demo', { kind: 'zone', ref: 'past' }), { expired: 0 }) + assert.equal(store.log.filter((e) => e.detail.resource === 'zone:past').length, 1) +}) + +test('a row past its until that the game still has stays in force', async () => { + // A deadline is the game's promise, not core's clock: a zone the game has not + // erased yet is still up, whatever core's time says. + stubExpiry() + registerAction({ reconcile: async () => ({ ok: true, inForce: ['late'] }) }) + addStep(1, 'demo.spawn') + addResource({ kind: 'zone', ref: 'late', lease_until: new Date('2026-10-22T14:00:00Z') }) + + const summary = await cleanup.reconcileModule('demo', new Date('2026-10-22T14:06:35Z')) + assert.equal(summary.inForce, 1) + assert.equal([...store.rows.values()][0].status, 'confirmed') +}) + +test('a reconcile that expires the last row of a finished run completes its cleanup', async () => { + stubExpiry() + registerAction({ reconcile: async () => ({ ok: true, inForce: [] }) }) + addStep(1, 'demo.spawn') + addResource({ kind: 'zone', ref: 'x', lease_until: new Date('2026-10-22T14:00:00Z') }) + store.cleanupStatus = 'incomplete' + + await cleanup.reconcileModule('demo', new Date('2026-10-22T14:06:35Z')) + assert.equal(store.cleanupStatus, 'complete') +}) + test('the last row of a finished run expiring completes an incomplete cleanup', async () => { stubExpiry() addResource({ kind: 'zone', ref: 'x' }) diff --git a/server/test/eventModuleContract.test.js b/server/test/eventModuleContract.test.js index 8117e8a..50e19d7 100644 --- a/server/test/eventModuleContract.test.js +++ b/server/test/eventModuleContract.test.js @@ -694,14 +694,24 @@ test('inForce() is optional, and a fourth question rather than a fourth spelling }]) }` + // + // Each in a modules directory of its own, because a site runs one module (W19): + // side by side, `leaseb` and `leasec` would be refused for `leasea`'s sake. + const fresh = () => { + tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'rg-events-module-')) + } + const without = loadModule('leasea', lease('leasea', '')) assertRegistered(without) assert.equal(registries.eventLease('leasea.rate.gain').inForce, null) + fresh() const withIt = loadModule('leaseb', lease('leaseb', 'async inForce() { return { ok: true, held: true } },')) assertRegistered(withIt) assert.equal(typeof registries.eventLease('leaseb.rate.gain').inForce, 'function') + fresh() + // And a declaration that is present but not callable fails the module rather // than being ignored -- the same posture the other three take. A module that // meant to answer and cannot is a module whose leases would silently never be diff --git a/server/test/eventSpec.test.js b/server/test/eventSpec.test.js index df65967..14876f7 100644 --- a/server/test/eventSpec.test.js +++ b/server/test/eventSpec.test.js @@ -477,3 +477,20 @@ test('the size bounds hold', () => { assert.equal(result.ok, false) assert.match(result.errors.join('\n'), new RegExp(`at most ${spec.MAX_PHASES} phases`)) }) + +test('a fingerprint is the plan, not the order its keys were written in', () => { + // W5: publish compares the fingerprint of the spec it freezes with the one a + // draft dry run kept. A spec that round-trips through a JSON column with its + // keys reordered is the same plan and must carry its pass over. + const a = { schedule: { kind: 'manual' }, phases: [{ key: 'main', label: 'Main', steps: [] }] } + const b = { phases: [{ steps: [], label: 'Main', key: 'main' }], schedule: { kind: 'manual' } } + assert.equal(spec.fingerprint(a), spec.fingerprint(b)) + assert.match(spec.fingerprint(a), /^[0-9a-f]{64}$/) + + // Array order IS the plan: steps run in it. + const c = { schedule: { kind: 'manual' }, phases: [{ key: 'main', label: 'Main', steps: [1, 2] }] } + const d = { schedule: { kind: 'manual' }, phases: [{ key: 'main', label: 'Main', steps: [2, 1] }] } + assert.notEqual(spec.fingerprint(c), spec.fingerprint(d)) + // And a key left undefined is the key left out, as JSON has it. + assert.equal(spec.fingerprint({ ...a, starts: undefined }), spec.fingerprint(a)) +}) diff --git a/server/test/eventsAdmin.test.js b/server/test/eventsAdmin.test.js index 4b46902..e87c7aa 100644 --- a/server/test/eventsAdmin.test.js +++ b/server/test/eventsAdmin.test.js @@ -152,6 +152,12 @@ function installStubs() { Object.assign(existing, { state: 'ready', current_version_id: versionId, updated_by: userId }) } } + definitionsDb.markDraftVerified = async (id, { at = new Date(), userId = null, hash }) => { + const existing = store.definitions.get(id) + if (existing) { + Object.assign(existing, { draft_verified_at: at, draft_verified_by: userId, draft_verified_hash: hash }) + } + } definitionsDb.archive = async (id, userId) => { const existing = store.definitions.get(id) if (existing) Object.assign(existing, { state: 'archived', updated_by: userId }) @@ -998,6 +1004,106 @@ test('a draft is verified against its working spec, and the pass is not recorded assert.equal(res.body.recorded, false) assert.equal(res.body.report.ok, true) assert.equal(res.body.report.steps, 1) + // Kept for publish to compare against instead (W5). + assert.equal(res.body.draftRecorded, true) +}) + +// W5 (Rust PLAN_LAST_FIXES): an author who dry-ran the draft and then published +// found v1 "not dry run", and its scheduled and trigger starts held, until they +// ran the same check again. + +test('a dry run that passed on the draft counts for the version published from it', async () => { + const { body } = await createDraft() + const id = body.event.id + await call(ctrl.verify, { user: EDITOR, params: { id: String(id) } }) + const passedAt = store.definitions.get(id).draft_verified_at + + const published = await call(ctrl.publish, { user: ADMIN, params: { id: String(id) } }) + assert.equal(published.statusCode, 200) + assert.equal(published.body.verified, true) + + // The reviewer and time are the dry run's, not the publisher's and now. + const version = store.versions.get(published.body.versionId) + assert.equal(version.verified_by, EDITOR.id) + assert.equal(version.verified_at, passedAt) + const after = await call(ctrl.get, { user: ADMIN, params: { id: String(id) } }) + assert.ok(after.body.event.currentVersionVerifiedAt) +}) + +test('an edit after the draft dry run means nothing carries over', async () => { + const { body } = await createDraft() + const id = body.event.id + await call(ctrl.verify, { user: EDITOR, params: { id: String(id) } }) + await call(ctrl.update, { + user: EDITOR, + params: { id: String(id) }, + body: draftBody({ + spec: { + schedule: { kind: 'manual' }, + phases: [{ key: 'main', label: 'Main', steps: [announceStep('The gates open at midnight.')] }], + }, + }), + }) + + const published = await call(ctrl.publish, { user: ADMIN, params: { id: String(id) } }) + assert.equal(published.body.verified, false) + assert.equal(store.versions.get(published.body.versionId).verified_at, null) +}) + +test('a title edit after the draft dry run still carries it over, because the plan is the same', async () => { + // The fingerprint is the spec's, and the title is not part of the plan a dry + // run checks. + const { body } = await createDraft() + const id = body.event.id + await call(ctrl.verify, { user: EDITOR, params: { id: String(id) } }) + await call(ctrl.update, { user: EDITOR, params: { id: String(id) }, body: { title: 'Renamed' } }) + + const published = await call(ctrl.publish, { user: ADMIN, params: { id: String(id) } }) + assert.equal(published.body.verified, true) +}) + +test('a draft dry run with findings keeps nothing to carry over', async () => { + const { body } = await createDraft() + const id = body.event.id + await call(ctrl.saveAction, { user: ADMIN, body: { actionId: 'core.announce', enabled: false } }) + const res = await call(ctrl.verify, { user: EDITOR, params: { id: String(id) } }) + assert.equal(res.body.report.ok, false) + assert.equal(res.body.draftRecorded, false) + + await call(ctrl.saveAction, { user: ADMIN, body: { actionId: 'core.announce', enabled: true } }) + const published = await call(ctrl.publish, { user: ADMIN, params: { id: String(id) } }) + assert.equal(published.body.verified, false) +}) + +test('the carried pass is written on the occurrences that publish re-pins', async () => { + // v1 published and scheduled, then v2 published from a draft that was + // dry-run first: the held occurrence moves to v2 and says why it may start. + const { body } = await createDraft() + const id = body.event.id + await call(ctrl.publish, { user: ADMIN, params: { id: String(id) } }) + await call(ctrl.startRun, { user: ADMIN, params: { id: String(id) }, body: { scheduledFor: '2099-11-01T18:00:00.000Z' } }) + // A ready definition dry-runs its published version, so the draft pass is + // planted the way a dry run on the draft would have left it. + const edited = draftBody({ + spec: { + schedule: { kind: 'manual' }, + phases: [{ key: 'main', label: 'Main', steps: [announceStep('Later.')] }], + }, + }) + await call(ctrl.update, { user: EDITOR, params: { id: String(id) }, body: edited }) + const spec = require('../src/events/spec') + const saved = store.definitions.get(id).spec + const checked = spec.validate(saved, { knownActionIds: spec.actionIdsIn(saved) }) + store.definitions.get(id).draft_verified_hash = spec.fingerprint(checked.spec) + store.definitions.get(id).draft_verified_by = EDITOR.id + + const published = await call(ctrl.publish, { user: ADMIN, params: { id: String(id) } }) + assert.equal(published.body.repinned, 1) + assert.equal(published.body.verified, true) + const line = store.log.find((e) => e.kind === 'version.verified') + assert.ok(line, 'the re-pinned occurrence says it may now start') + assert.equal(line.detail.fromDraft, true) + assert.equal(line.detail.versionId, published.body.versionId) }) test('a ready definition is verified against the version that would actually run, and the pass IS recorded', async () => { diff --git a/server/test/moduleLifecycle.test.js b/server/test/moduleLifecycle.test.js index 434b0f3..6f45bad 100644 --- a/server/test/moduleLifecycle.test.js +++ b/server/test/moduleLifecycle.test.js @@ -143,10 +143,9 @@ beforeEach(() => { // ── The happy path ───────────────────────────────────────────────────────── -test('every module is recorded, booted with its ctx and marked started, in scan order', async () => { +test('the site\'s module is recorded, booted with its ctx and marked started', async () => { const file = path.join(tmpRoot, 'log.txt') writeModule('aaa', { boot: '', log: file }) - writeModule('bbb', { boot: '', log: file }) const loader = freshLoader(tmpRoot) const model = fakeModel() @@ -155,15 +154,35 @@ test('every module is recorded, booted with its ctx and marked started, in scan // The reconcile order IS the design (§2.4): clear the last boot's outcomes // first, so what is on display afterwards is what this boot did. assert.equal(model.calls[0], 'beginBoot') - assert.deepEqual(model.calls.slice(1, 3), ['recordInstalled:aaa', 'recordInstalled:bbb']) - assert.deepEqual(noted(file), ['boot:aaa:ctx', 'boot:bbb:ctx']) + assert.equal(model.calls[1], 'recordInstalled:aaa') + assert.deepEqual(noted(file), ['boot:aaa:ctx']) assert.equal(stateOf(loader, 'aaa').state, 'started') - assert.equal(model.rows.get('bbb').state, 'started') + assert.equal(model.rows.get('aaa').state, 'started') // §2.4's metadata refresh: the row carries what the admin screen shows. assert.equal(model.rows.get('aaa').name, 'Module aaa') assert.equal(model.rows.get('aaa').version, '1.2.3') }) +test('a second module on the volume gets a failed row and is never booted', async () => { + // W19. Hand-copied beside the site's module, it still needs a row: without one + // Admin → Modules could not say why it is not running. + const file = path.join(tmpRoot, 'log.txt') + writeModule('aaa', { boot: '', log: file }) + writeModule('bbb', { boot: '', log: file }) + const loader = freshLoader(tmpRoot) + const model = fakeModel() + + await lifecycle.boot({ modules: loader, model }) + + assert.deepEqual(noted(file), ['boot:aaa:ctx']) + assert.equal(model.rows.get('aaa').state, 'started') + const refused = model.rows.get('bbb') + assert.equal(refused.state, 'startup_failed') + assert.equal(refused.failureStage, 'manifest') + assert.match(refused.failureReason, /already runs the module "aaa", and a site runs one module/) + assert.equal(refused.name, 'Module bbb') +}) + test('a hand-placed directory gets a row with no provenance', async () => { // §2.5 keeps a directory dropped on the volume by hand a supported install. // Without a row it could never be disabled, and nothing could report it. @@ -193,23 +212,19 @@ test('a module with no onBoot still reaches started', async () => { // ── Failure is a state ───────────────────────────────────────────────────── -test('an onBoot that throws fails its own module and no one else', async () => { +test('an onBoot that throws is a failed state, not a failed boot', async () => { const file = path.join(tmpRoot, 'log.txt') - writeModule('aaa', { boot: '', log: file }) writeModule('bbb', { boot: 'throw new Error("cache warm-up failed")', log: file }) - writeModule('ccc', { boot: '', log: file }) const loader = freshLoader(tmpRoot) const model = fakeModel() - await lifecycle.boot({ modules: loader, model }) + await assert.doesNotReject(() => lifecycle.boot({ modules: loader, model })) + assert.deepEqual(noted(file), ['boot:bbb:ctx']) assert.equal(stateOf(loader, 'bbb').state, 'startup_failed') assert.equal(stateOf(loader, 'bbb').stage, 'boot') assert.match(stateOf(loader, 'bbb').reason, /cache warm-up failed/) assert.equal(model.rows.get('bbb').failureStage, 'boot') - // The one that matters: the module AFTER the failure still booted. - assert.deepEqual(noted(file), ['boot:aaa:ctx', 'boot:bbb:ctx', 'boot:ccc:ctx']) - assert.equal(stateOf(loader, 'ccc').state, 'started') }) test('a module whose onBoot failed keeps its URLs and answers 503', async () => { @@ -333,25 +348,28 @@ test('a process that never scanned writes nothing at all', async () => { // ── Shutdown ─────────────────────────────────────────────────────────────── -test('shutdown runs started modules in reverse order and skips the rest', async () => { - const file = path.join(tmpRoot, 'log.txt') - writeModule('aaa', { boot: '', shutdown: '', log: file }) - writeModule('bbb', { boot: 'throw new Error("no")', shutdown: '', log: file }) - writeModule('ccc', { boot: '', shutdown: '', log: file }) - const loader = freshLoader(tmpRoot) +test('shutdown runs a started module and skips one whose boot failed', async () => { + for (const [boot, expected] of [ + ['', ['shutdown:aaa']], + // Its onBoot threw, so it has a half-built world that its onShutdown was + // never written to tear down. + ['throw new Error("no")', []], + ]) { + tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'rg-lifecycle-')) + const file = path.join(tmpRoot, 'log.txt') + writeModule('aaa', { boot, shutdown: '', log: file }) + const loader = freshLoader(tmpRoot) - await lifecycle.boot({ modules: loader, model: fakeModel() }) - fs.writeFileSync(file, '') // only the shutdown half is under test - await lifecycle.shutdown({ modules: loader }) + await lifecycle.boot({ modules: loader, model: fakeModel() }) + fs.writeFileSync(file, '') // only the shutdown half is under test + await lifecycle.shutdown({ modules: loader }) - // Reverse of boot order, and `bbb` absent: its onBoot threw, so it has a - // half-built world that its onShutdown was never written to tear down. - assert.deepEqual(noted(file), ['shutdown:ccc', 'shutdown:aaa']) + assert.deepEqual(noted(file).filter(Boolean), expected) + } }) test('a hook that hangs costs its budget, not the shutdown', async () => { const file = path.join(tmpRoot, 'log.txt') - writeModule('aaa', { boot: '', shutdown: '', log: file }) writeModule('zzz', { boot: '', shutdown: 'await new Promise(() => {})', log: file }) const loader = freshLoader(tmpRoot) @@ -361,15 +379,14 @@ test('a hook that hangs costs its budget, not the shutdown', async () => { const started = Date.now() await lifecycle.shutdown({ modules: loader, budgetMs: 50 }) - // zzz never returns; it is abandoned and aaa still gets its turn. The - // alternative is a host where `systemctl stop` hangs until SIGKILL. - assert.deepEqual(noted(file), ['shutdown:zzz', 'shutdown:aaa']) + // zzz never returns, and is abandoned. The alternative is a host where + // `systemctl stop` hangs until SIGKILL. + assert.deepEqual(noted(file), ['shutdown:zzz']) assert.ok(Date.now() - started < 2000, 'shutdown must not wait on a hung hook') }) -test('a hook that throws does not stop the ones behind it', async () => { +test('a hook that throws does not fail the shutdown', async () => { const file = path.join(tmpRoot, 'log.txt') - writeModule('aaa', { boot: '', shutdown: '', log: file }) writeModule('zzz', { boot: '', shutdown: 'throw new Error("close failed")', log: file }) const loader = freshLoader(tmpRoot) @@ -377,7 +394,7 @@ test('a hook that throws does not stop the ones behind it', async () => { fs.writeFileSync(file, '') await assert.doesNotReject(() => lifecycle.shutdown({ modules: loader })) - assert.deepEqual(noted(file), ['shutdown:zzz', 'shutdown:aaa']) + assert.deepEqual(noted(file), ['shutdown:zzz']) }) // ── stop(): the admin panel's Disable ────────────────────────────────────── @@ -385,10 +402,9 @@ test('a hook that throws does not stop the ones behind it', async () => { // Phase 4, §2.7.2 decision 3. Phase 2's disable moved a record and left the // module running; the whole point of these tests is that it no longer does. -test('stop runs that one module\'s onShutdown and leaves the others alone', async () => { +test('stop runs the module\'s onShutdown and disables it', async () => { const file = path.join(tmpRoot, 'log.txt') writeModule('aaa', { boot: '', shutdown: '', log: file }) - writeModule('bbb', { boot: '', shutdown: '', log: file }) const loader = freshLoader(tmpRoot) const model = fakeModel() @@ -398,10 +414,8 @@ test('stop runs that one module\'s onShutdown and leaves the others alone', asyn const result = await lifecycle.stop('aaa', { modules: loader, model }) assert.deepEqual(result, { stopped: true, error: null }) - // Only aaa. This is the difference from shutdown(), which runs everything. assert.deepEqual(noted(file), ['shutdown:aaa']) assert.equal(stateOf(loader, 'aaa').state, 'disabled') - assert.equal(stateOf(loader, 'bbb').state, 'started', 'the other module keeps running') assert.equal(model.rows.get('aaa').state, 'disabled') }) diff --git a/server/test/moduleLoader.test.js b/server/test/moduleLoader.test.js index 6e69d4b..ddb56a7 100644 --- a/server/test/moduleLoader.test.js +++ b/server/test/moduleLoader.test.js @@ -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', () => { diff --git a/server/test/moduleSchema.test.js b/server/test/moduleSchema.test.js index 6dffd0e..457364e 100644 --- a/server/test/moduleSchema.test.js +++ b/server/test/moduleSchema.test.js @@ -150,23 +150,18 @@ test('fragments are split exactly the way core schema.sql is', async () => { // ── 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);' }) +test('a fragment that throws fails its module, not the boot', async () => { 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 }) + // seedDefaults(), and a bad module must not stop the site coming up. + await assert.doesNotReject(() => 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) + assert.equal(rec.ran.length, 1) }) test('a fragment stops at its first failing statement', async () => { @@ -217,15 +212,13 @@ 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/) + assert.equal(rec.ran.length, 0) }) // ── The seed script ──────────────────────────────────────────────────────── -- 2.49.1