feat(modules)!: the module system cutover — a game-agnostic core reaches main #150
Reference in New Issue
Block a user
No description provided.
Delete Branch "edge"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The cutover. Decision 11 of
docs/website/MODULE_SYSTEM.md§2.9: the whole module-system workstream has lived onedgesince 2026-08-10 precisely so this could happen once. 52 commits, phases 0–5, every one of them already reviewed and merged individually.mainis a strict ancestor ofedge— no conflicts, nothing to resolve.What changes on
mainRunic Gateway stops being a UO/ServUO platform and becomes a game-agnostic one. Core keeps the architecture — sidecar → website → browser — and loses the game. Everything UO-specific (routes, tables, screens, nav, the uo-link connection) now lives in
RunicGateway/Module-uoand arrives as an installable module.MODULE_API_VERSIONWhat this does to a live deployment
Checked against the diff rather than assumed, because this is the merge that rolls production:
DROP TABLE, anywhere. Core stops creating the 27shard_*tables but never drops them, and the module's fragment isCREATE TABLE IF NOT EXISTS— so an existing instance keeps its shard data and its encrypteduo_link_configrow across the extraction. The module recreates nothing.announce_jobs→announce_job_legscolumn drop from phase 2 PR 4, guarded oninformation_schemaand idempotent. Already verified against the liveuomm-db— 3 legacy jobs migrated faithfully, 3 replays, no dupes.Accepted breaks, both deliberate
gameAccountSignupmoved off/public/settingsonto the module's/public/shard/features; the app still reads the old place and the field defaultsfalse, so it quietly stops offering game-account creation.docs/android/PLAN.mdhad gated the cutover on fixing this first — the org lead released that gate (2026-08-12): the app needs a broader pass than this one field, and holding the cutover for it buys nothing. Recorded, not forgotten.to, and the UO paths moved — so ordering, labels, hidden flags and any dropdown grouping of those rows are lost and get set again from Admin → Navigation. Accepted in phase 3 slice 3.Follow-ups, already scoped
edge(it is cut fresh next time it is needed).MODULE_SYSTEM.md's banner still says "Phase 2 PRs 1–7 of 9".Integration-kit'sci/core-ref.jsonpins branchedge, which this merge retires.Risk
The highest-risk merge in this workstream by construction, and the reason for every gate in front of it: 52 individually-reviewed commits, an exit criterion proved per phase, and full CI on this PR. What it cannot prove is the deploy itself, which the org lead is handling.
🤖 AI-assisted: written with Claude Code (Claude Opus 5).
Phase 2 PR 2 of docs/website/MODULE_SYSTEM.md 2.7. Adds server/src/modules/{loader,semver,version}.js: the synchronous scan of MODULES_DIR, manifest validation, prefix and table-name collision rejection, per-module try/catch and the mount into the three tier routers behind the MODULE_API.md 4.5 dispatch guard. Two decisions the contract left open, both now written up there: - The load trigger is one explicit modules.load(tierRouters) call in app.js, not a lazy scan (API 7.6). Accessors throw until it has run, because "no modules installed" is a real answer a caller must not be handed by accident. - Whether core owns a prefix is asked of the live tier routers via express's own layer.match(), skipping root-mounted layers, rather than a hardcoded table -- the spike's was already stale when written (API 4.3). Mounting is a second pass after every module is validated. Doing it inside the scan loop makes the first module's layers indistinguishable from core's, so the second module claiming a taken prefix is told it collided with core and the module-versus-module check is unreachable. registerExtension/NotificationStreams/AnnounceLeg and onBoot/onShutdown throw "not available until phase 2 PR 4/5" rather than no-op; an accepting stub would let a module believe it had registered something. No schema replay, no boot dispatch, no installed_modules reconcile -- those are PRs 3 and 5, and until PR 5 a record's state is in memory only. No module ships on the volume, so nothing an operator or client can see changes: 842 tests pass, routes.manifest.json is unchanged at 229 routes and swagger-output.json regenerates byte-identical. Co-Authored-By: Claude <noreply@anthropic.com>Phase 2, PR 6 of docs/website/MODULE_SYSTEM.md 2.7 — the first module-system URL a client can see. The SPA and the Android app feature-detect against the capabilities a module declares; the shape is settled in MODULE_API.md 2.9. Four decisions, and what is absent from the payload is most of the design: * started modules only. A module that is disabled or failed to load is ABSENT, exactly as 4.4 already leaves its routes and its nav absent, so a client renders a site without that capability rather than advertising one that 503s. * no state, failure_stage or failure_reason. Where a module broke belongs to the admin Modules screen, and the reason is an exception string from inside core — not anonymous-visitor business. * no client chunk URL. htmlShell injects a script tag per started module (3.1.3), so the browser is handed the tag rather than a URL to fetch. This endpoint feature-detects; it does not load. MODULE_SYSTEM 2.6 step 4 is amended to match (API 6.7). * no siteMode gate and no database — the same class as /public/status and /public/version, so a client can still feature-detect during maintenance. It is a capability router of its own rather than a fifth singleton in site.router.js, and that is load-bearing: the loader's prefix-collision probe reads the live tier stack and skips root-mounted layers, because a use('/', ...) matches every path. A route inside the root-mounted site router would be invisible to it — mounting use('/modules', ...) is what makes "no module may claim /modules" a rule the loader enforces. 910 tests pass (+9, every one on the boundary — what must NOT appear). routes.manifest.json gains exactly the one route and routes.guards.json records it with an empty gates list, which is itself the assertion that it is ungated. Co-Authored-By: Claude <noreply@anthropic.com>Phase 2, PR 7 of docs/website/MODULE_SYSTEM.md 2.7 — the client half's delivery. A module's prebuilt chunk is served, injected, handed core's React and its UI kit, and its routes are rendered by App.jsx. The registry is empty on a bare core, so nothing an operator can see changes. Client: - modules/registry.js — registerRoutes/registerNav/registerFeatureProvider, with the URL namespace written by core, never by the module - modules/shared.js — window.__rg: React, react-dom/client, react-router-dom, react/jsx-runtime, the registry, the seven-member UI kit and the request primitive, frozen - App.jsx reads routesFor for all three areas; nav consumption is PR 8 - main.jsx publishes the global, then mounts on DOMContentLoaded Server: - the loader validates client.entry and publishes clientChunks() and clientEntryUrls(); an entry in the module root is rejected, because the directory it sits in is what gets served - app.js mounts each chunk at /modules/<id>/ behind the module's state guard with no-cache; anything else under /modules is a 404, not the SPA shell - htmlShell injects the tag before </body>, so core's bundle runs first wherever a bundler puts it Found by loading a real chunk in a browser, and fixed here: core mounted before any module chunk had evaluated, because document.readyState during a deferred script is 'interactive', not 'loading'. Every test passed against that build. The smoke is written down in MODULE_API.md 7.7. 933 server tests (+23), 123 client tests (+14). routes.manifest.json unchanged at 230 routes; the OpenAPI spec regenerates byte-identical. Co-Authored-By: Claude <noreply@anthropic.com>Core's half of the slice that closes phase 3. Two things: the request-time fragment merge core has owed since phase 1, and the last of core's UO copy. **The merge (MODULE_API.md §6.1a).** `swagger-output.json` is core's own routes and cannot be anything else — it is generated on a developer's machine and committed, so it must come out the same regardless of what they had checked out, and a module arrives on a volume long after the image was built. Module routes therefore reach the document at request time, from the `swagger-fragment.json` each module ships: `swagger/docsSpec.js` merges the fragments of STARTED modules over the committed spec, cached on a new loader state version and rebuilt when a module's state moves. Until now neither half existed. `swagger/mergeSpec.js` named the request-time caller in its header and that caller was never written, so the 72 routes module-uo serves were in no OpenAPI spec at all — core's standing rule ("never ship a route that isn't in the spec") broken by the extraction rather than by a route. Core wins every key collision, `swagger-output.json` is never mutated (it is a require()d JSON module — one in-place merge would be permanent AND cumulative), and a fragment that is missing or unreadable costs that module its paths and nothing else. The Swagger UI is now built per request for the same reason the JSON is: bound once at require time it would show core's routes for the life of the process while /api/docs.json showed the merged set. **The last of core's UO copy** (slice 4 deferred it; §5.2's check reads code, not prose, so none of this was caught): - 31 UO schemas and 4 UO tags in `swagger/swagger.js`, describing routes core has not served since slice 1 — 578 lines. They moved to module-uo, namespaced `Uo…`, and arrive back through the merge on an instance that installs it. - `info.description` said "a private Ultima Online shard". - README.md's 48 UO mentions, including the architecture diagram and the whole `## Shard integration (uo-link)` section, now `## Modules`. - `TOWNCRIER_DURATION_SEC` and `UOLINK_*` in the two `.env.example`s: read by the module, not by core, and documented in the module's README instead. **Two dropped annotations, and the reason nobody knew.** swagger-autogen reports an annotation it cannot parse and then prints Success in green, having skipped it. `npm run swagger` now captures its diagnostics and fails — which immediately found `POST /api/v1/admin/invites` and `POST /api/v1/auth/invite/:token/accept` documented with an EMPTY request body, both since the day they were written. Fixing the tag list also cleared five tags used by routes but never declared (`Admin · Email`, `Admin · Invites`, `Admin · Moderation`, `Admin · Pages`, `Auth · Me`) — the same defect class, in the other direction. - 646 server tests (+9), 157 client tests unchanged - routes.manifest.json unchanged (158 public + 2 internal); check:modules clean - swagger-output.json: 128 paths, 69 schemas, 0 orphan tags, 0 orphan schemas - verified against a real boot with module-uo installed: 197 merged paths (128 core + 69 module), all four module tags, 31 Uo schemas, no dangling $refs, /api/docs renders the module's operations with zero console errors Refs: docs/website/MODULE_API.md §2.8, §6.1a; MODULE_SYSTEM.md §2.7.1 Co-Authored-By: Claude <noreply@anthropic.com>The consumer half of a release module-uo's CI has been publishing since phase 3 closed. Before this, core had the installed_modules provenance columns and no code that could ever fill them: nothing fetched, verified, unpacked, removed or purged anything, and there was no admin route at all. Adds modules/archive.js, modules/install.js, schema.runPurge(), lifecycle.stop(), loader.stopHook(), and /api/v1/admin/modules with eight routes. 797 server tests (+76), manifest 158 -> 166 + 2 internal, OpenAPI gains 8 operations and loses nothing. Reject, never sanitise ---------------------- The download is the easy part: an https-only allowlist re-checked on every redirect hop, a declared sha256 compared against the bytes that arrived, and a byte cap. Unpacking is where the archive chooses the filenames, and core writes into a directory bind-mounted from the host, so an escape is not confined to the container. archive.js inspects the whole archive before a byte is unpacked and refuses absolute and drive-absolute paths, `..` segments, NUL bytes, backslashes, anything that is not a regular file or a directory, more than one top-level entry, and anything over the entry or byte caps. Refusing symlinks and hardlinks outright is what keeps this off the majority of node-tar's published advisories rather than depending on the library to contain them. That two-pass shape is load-bearing, and it was measured rather than assumed: extracting an archive whose fourth member escapes upward throws under node-tar 7.5.22 -- and leaves the first three members on disk. The loader scans that directory at require time on the next boot, so a half-unpacked module is a module. Everything therefore happens in a scratch directory that is removed on any failure, and the move into place is the last step. `tar` is pinned to ^7.5.22 rather than the ^6 that installs by default: 6.x is flagged critical, and reading the advisory list is what the file's header now says out loud -- almost all of it is hardlink or symlink traversal and PAX header interpretation differentials, which is exactly this feature's threat model. Two things the plan had wrong ----------------------------- The bundle's top-level directory is `module-uo-<version>`, not the module id -- so "the top-level name must equal the id" was checked against nothing real. The extractor strips that level instead, because its name belongs to whoever published the bundle and the directory it lands in is core's. What is checked instead is the unpacked module.json: a manifest promising `uo` and delivering something else is refused rather than installed under the name it promised. And purge cannot be a follow-up action (decision 5): purge.sql lives inside the directory uninstall deletes. It is offered in the uninstall flow and as a standalone action on a still-installed module, and the standalone one refuses unless the module is already disabled -- dropping tables under something that is still serving leaves it answering out of a world that no longer exists. Disable now means stopped ------------------------- lifecycle.stop() dispatches that one module's onShutdown before flipping the guard, so a module an operator switches off actually releases its sockets and closes its streams instead of merely becoming unreachable. The hook runs first and the state moves after it, because while onShutdown runs the module is still `started` and that is the only state in which its routes and the world it is tearing down agree. A hook that throws does not stop the disable -- the opposite of the boot path's rule, and deliberately. Enable is not its mirror and there is no start(id) beside it. There is no onBoot re-dispatch and the hooks were never promised re-entrant, so enable moves the row and the restart route starts it. A test pins that enable does not touch the loader, because "fixing" it is a one-line change that would put a module with closed sockets back on the nav. Restart raises SIGTERM against its own process rather than calling the shutdown path directly, so server.js's handler stays the one graceful-shutdown path and this route cannot drift from it. The allowlist bootstraps from MODULE_SOURCE_HOSTS into a settings row and is admin-managed after that (decision 6); seedDefault is INSERT IGNORE, so changing the variable on an existing deployment is a no-op by design. An empty list forbids every install rather than allowing every host -- the safe direction for a value someone might blank by accident. Verified against the real v0.3.0 release ---------------------------------------- Not a fixture: fetched the published install manifest over the real Gitea host and its redirect chain, verified the sha256, inspected and unpacked the 252,517-byte artifact to 82 files, and then booted core against the result -- the module registered its five mounts, seven streams and eight capabilities and resolved its client chunk, with no scratch directory left behind. Two defects this slice's own tooling caught, both of which had already been written down as classes: - the controller destructured runPurge at require time, capturing the function rather than the module, which made the one dependency whose ORDER matters the one that could not be substituted; - two swagger annotations carried an apostrophe inside a quoted string, dropped silently by swagger-autogen before slice 5 taught it to fail loudly. Co-Authored-By: Claude <noreply@anthropic.com>Standing the slice-2 screen up against a live server and installing the published module-uo v0.3.0 through it found three things, none of which any unit test in this repo could have caught. Two of them are older than this phase. 1. The boot refresh nulled every install's provenance -------------------------------------------------------- `installed_modules.source` and `.sha256` exist so the admin panel can say where a module came from. They never survived a restart. `lifecycle.boot()` re-records every scanned module with no source and no sha256 -- correctly, because a scan finds a directory and never where it came from -- and `upsert` assigned both columns unconditionally. So an install's provenance lasted exactly until the restart that install asked for, and the screen then described a module installed from a URL as "placed on the volume by hand". Verified live: install, restart, provenance gone. Nothing could have caught it before now. Phase 4 wrote the first non-null value these columns had ever had, so lifecycle.js's comment asserting that "recordInstalled leaves what it is not given" described an intention rather than the statement below it -- and modules.model.test.js's fake reproduced the defect faithfully, assigning unconditionally just like the SQL. Fixed with COALESCE(VALUES(col), col): a value overwrites, a NULL leaves what is there. The fake now matches, and two tests pin both directions -- a boot refresh must not wipe it, and a re-install from a new URL must still replace it, or the column would become write-once and an upgrade would for ever show where the first version came from. 2. The restart killed the server on Windows instead of stopping it ------------------------------------------------------------------ The route called `process.kill(process.pid, 'SIGTERM')` to reach server.js's graceful-shutdown handler. That works on Linux. **Windows has no POSIX signals, and Node documents SIGTERM there as unconditional termination of the target process** -- so on a Windows host the restart killed the server outright: no module onShutdown, no listener close, no pool close, no log flush. Observed exactly that: the process was gone and the shutdown handler had logged nothing at all. `process.on('SIGTERM', ...)` is an ordinary EventEmitter listener, so `process.emit('SIGTERM')` reaches the same handler on every platform without involving the OS. One shutdown path, still; it just gets there by an event. Deployment is Linux containers and would never have shown this. Development is not, and neither is the smoke that found it. The test was worse than useless: it stubbed `process.kill` and asserted it had been called with SIGTERM, which is precisely the call whose MEANING differs by platform. It now waits for the SIGTERM EVENT -- what server.js is actually subscribed to -- so a pass here means the handler would run. 3. `present()` did not publish the running version -------------------------------------------------- An upgrade writes new files and a new row while the old code stays loaded, so the row's version is a promise about the next boot rather than a description of this one. Adds `liveVersion` from the loader beside `liveState`, so the screen can tell the two apart instead of reporting the new version as running. 723 server tests (+2), manifest and OpenAPI both unchanged. Co-Authored-By: Claude <noreply@anthropic.com>MODULES declares the module set a deployment runs, one entry per module as `<id>@<version>=<install manifest URL>`, and the container arrives at it by itself (MODULE_SYSTEM.md §2.7.2 decision 4). A module already unpacked at the declared version is a no-op that makes NO network call, so a restart with the network down comes up unchanged; anything else goes through install.js — same allowlist, same sha256, same inspect-then-extract — and install() now takes an `expect: {id, version}` so a URL resolving to another module or version is refused while it is still only a manifest. Resolution runs inside start(), between the seed and the require of app.js: the seed is where the host allowlist setting comes from, and the require is what scans the volume. That buys it the database, so a compose-installed module gets the same provenance columns an admin install writes. A failure is logged and carried, never fatal — an unreachable release host must not take the site down. The declaration owns what is on the volume; the row owns whether a module runs, so uninstalling a declared module returns its files at the next start and leaves it disabled. The admin list gains that as a fourth source (declared / declaredVersion / declaredError), because a declared module that failed to resolve has no row, no directory and nothing mounted. Deferring the app require moved core's schema ahead of the volume scan, and the module schema-fragment replay was wired to core's schema — so every installed module silently got no tables. Invisible to the suite (each one stubs the loader or the pool) and to a smoke on a database that already had the tables; found by booting against an empty one. ensureSchema() now takes `replayModules: false` for the one caller that scans later, server.js replays them itself after the require, and a bootOrder test pins the five steps in the only order they work in. 741 server tests (+18), 187 client (+5); manifest unchanged at 166 public + 2 internal, OpenAPI byte-identical. Co-Authored-By: Claude <noreply@anthropic.com>The Integration Kit's acceptance run (Phase 5 slice 3) put a cold agent in front of the kit alone and asked it to build a module for a second game. It built one that works — and its page rendered outside the site. PublicLayout supplies the chrome and not the body. Every core public page wraps its own content in `<div className="shell-... page-body">`: the centred column, the top and bottom padding, and — through `page-body { flex: 1 }` — the thing that pushes the footer to the bottom of the viewport. Nine of nine core pages do it, so the omission has never shown. A module cannot do it: it receives PublicLayout through the UI kit and those two class names appear in no contract. The result was a page at x=0 with the footer riding up under the content, which is the exact failure MODULE_API.md §3.4 says the kit exists to prevent. So the wrapper moves behind the component a module already has: <PublicLayout shell="narrow"> // or "mid" / "wide" `shell` is opt-in and omitting it is 1.4.0's behaviour exactly, so core's nine pages are untouched and keep their own wrapper. An unrecognised width falls back to narrow rather than to nothing — a module page at the wrong width still looks like the site; a page with no wrapper does not. 1.5.0 is minor, not major. §3.4 makes *changing* a kit component's props major because that breaks a call already written; adding an optional one breaks nothing. module-uo's `coreApi: "^1.3.0"` still resolves. The width map and its fallback live in client/src/lib/pageShell.js rather than in the component, for the reason lib/adminNav.js does: the client runner has no DOM and cannot import .jsx at all, so a rule inside a component is a rule no test can reach. Five tests cover it, including that every width it offers is a class theme.css actually defines — the contract now names those widths to module authors, so a rename has to fail here instead of silently in someone's page. Also from the same run: modules/shared.js called the UI kit "seven" members while exporting eight (§3.4's table has five rows because PageState contributes three), and its note said AdminPage "appears in §3.4's table" when the table dropped it in Phase 2 PR 7. 742 server + 192 client tests pass (+5). routes.manifest.json and the OpenAPI spec regenerate byte-identical — no route changed. Verified in a browser against the acceptance module (MODULE_API.md §7.7), which is the only place this seam is visible: the untouched build renders full-bleed, and shell="narrow" lands the page in the same column as core's own. Co-Authored-By: Claude <noreply@anthropic.com>