diff --git a/website/BACKEND_DESIGN.md b/website/BACKEND_DESIGN.md index bd88772..97b600b 100644 --- a/website/BACKEND_DESIGN.md +++ b/website/BACKEND_DESIGN.md @@ -365,7 +365,7 @@ A DB read never yields a usable reset link. See §4 `/auth/password/*`. | col | type | notes | |---|---|---| | user_id | INT NOT NULL FK→users(id) ON DELETE CASCADE | | -| stream_id | VARCHAR(64) NOT NULL | an id from the catalog (`config/notificationStreams.js`), validated on write | +| stream_id | VARCHAR(64) NOT NULL | an id from the catalog (`modules/registries.js` — core's plus every installed module's), validated on write | | created_at | DATETIME | | `PRIMARY KEY(user_id, stream_id)`. Subscriptions are per-user (applied to every device); a PUT @@ -793,14 +793,18 @@ registers device endpoints (`/auth/me/devices`); nothing is pushed unless subscr self-hosted **ntfy** endpoint (`utils/pushDispatch`); the app wakes and pulls the real, ownership- checked content over the authenticated API. Two producers fan out through the one publisher: the shard ingest dispatcher (`utils/shardIngest`, beside the SSE broadcast) for shard-derived streams, and the -create/publish-post path for `news.post`. The stream catalog + event→stream mapping is -`config/notificationStreams.js`. Security invariants: +create/publish-post path for `news.post`. The catalog is assembled at boot by +`modules/registries.js` from core's own streams (`config/coreStreams.js` — just `news.post`) plus +each installed module's; the shard streams and their event→stream mapping are +`config/shardStreams.js`, which belongs to module-uo and moves out with it +(MODULE_SYSTEM.md §1.8). Security invariants: - **Same public/admin split as the SSE feed.** Public streams are drawn *only* from the SSE `PUBLIC_KINDS` allowlist; a sensitive kind (audit/cheat/IP/login-attempt) can never produce a public push. - **Personal streams are owner-keyed.** `vendor.sale` / `house.idoc` / `account.login` are delivered only to the *owning* user's devices, resolved via `shardLinks` (the same ownership check as - `/player/shard/*`). + `/player/shard/*`) in `utils/shardPush.js`. `utils/pushDispatch.js` itself only publishes to a + stream id someone else resolved — it has no idea what a shard event is. - **SSRF guard.** A device `endpoint` is a client-supplied URL the server POSTs to, so registration and every publish validate it is HTTPS, non-private/loopback, and (when configured) on the shard's ntfy allow-set (`NTFY_BASE_URL` / `NTFY_ALLOWED_ORIGINS`). @@ -1130,7 +1134,7 @@ it picks, it fails open on one side.) | Nav | `GET /public/shard/features` returns only what the caller may reach, so the SPA never renders a link that would 403. Presentation only. | Config reads are cached ~5s, so admin changes take effect within seconds **including on already-open -streams**. `PUBLIC_KINDS` still exists and is still exported (`notificationStreams.js`) but is now +streams**. `PUBLIC_KINDS` still exists and is still exported (`utils/shardBroadcast.js`) but is now **derived** from the kind map rather than hand-maintained, so the two cannot drift. **`PUBLIC_KINDS` is a module-load constant and must not be used to answer "may this caller read this diff --git a/website/MODULE_API.md b/website/MODULE_API.md index 25b3010..3313c90 100644 --- a/website/MODULE_API.md +++ b/website/MODULE_API.md @@ -157,12 +157,21 @@ validated at once rather than at first use. ```js api.registerRoutes({ public: {...}, admin: {...}, player: {...} }) api.registerExtension(slot, router) -api.registerNotificationStreams({ streams, mapEvent }) -api.registerAnnounceLeg({ leg, dispatch, classify }) +api.registerNotificationStreams(streams) +api.registerAnnounceLeg({ leg, label, dispatch, classify }) api.onBoot(async (ctx) => {}) api.onShutdown(async () => {}) ``` +**Every call STAGES; nothing is committed until the module as a whole is known good.** A claim's +shape is checked at the call, so a malformed one throws with the registrant's own stack; whether the +name is *taken* can only be answered once the batch is complete, and is checked when the loader +commits it in its second pass. The consequence is the one that matters: a module that registers two +streams and then throws — or fails `checkDeclared` after `register()` returns — has left nothing +behind. A half-registered catalog would be worse than a missing one, because it is a subscribable +stream nothing will ever publish to. This is the registry-side twin of §4.3's second-pass mount rule, +and both exist for the same reason. + **`registerRoutes(mounts)`** — one `express.Router()` per prefix per tier: ```js @@ -194,15 +203,42 @@ The router receives `req.params.id` from the parent (`mergeParams: true`). Two m same slot is a collision and is rejected; core's own routes on the resource always win a path conflict. -**`registerNotificationStreams({ streams, mapEvent })`** — §1.8's push catalog. -`streams` is an array of `{ id, label, description, scope }` appended to core's catalog (ids are -namespaced `.` and rejected otherwise); `mapEvent(event) => streamId | null` is -called by core's dispatcher for events the module's own code publishes. +**`registerNotificationStreams(streams)`** — §1.8's push catalog. +An array of `{ id, label, description, personal, requiresLinkedAccount }` appended to core's catalog. +Ids are namespaced `.` and rejected otherwise, save for the seven grandfathered ones +in §6.4. -**`registerAnnounceLeg({ leg, dispatch, classify })`** — §1.8's news dispatcher. -`leg` is a namespaced id, `dispatch(post) => Promise` delivers, `classify(post) => boolean` -decides whether this leg wants the post. A leg that throws is retried by core's existing per-leg -retry and never blocks another leg. +Two amendments this signature carries, both settled 2026-08-10 with PR 4: + +- **`mapEvent` is gone.** The earlier signature took `{ streams, mapEvent }`, with core's dispatcher + calling `mapEvent(event) => streamId`. That was a leftover from before §1.8's push inversion was + settled: the module owns `fromShardEvent` outright and calls `ctx.push.publish(streamId, …)` with + an id it has already resolved, so core never needs a second way to get there. What core wants from + a module here is the catalog — for the subscribe endpoint, for validating a subscription write, and + for the personal/linked-account gate. It follows that the public-safety filter (a sensitive event + kind can never produce a *public* push) is module-internal; that is the right home, because the + kinds, the streams and the filter are then one file that moves together, rather than a rule in core + about data only the module defines. +- **Two booleans, not one `scope`.** The entry shape above is the response body of + `GET /auth/me/notifications/streams`, which a shipped Android client already reads + (`NotificationsDto.kt`). `scope` was never the wire shape. + +**`registerAnnounceLeg({ leg, label, dispatch, classify })`** — §1.8's news dispatcher. +`leg` is a namespaced id, `label` is what the admin panel shows, `dispatch(post) => Promise` +delivers, and `classify(result) => { outcome, error }` maps the client's result to +`done` / `retry` / `terminal`. A leg that throws is caught, classified as a retry, and never blocks +another leg. + +`label` is an addition: the panel used to hold a client-side `{ towncrier, discord }` label table, +which would have left a module's leg rendering as a bare id. It comes from the registration so a +module needs no client change. + +**Legs are rows, not columns.** `announce_jobs` carried a `towncrier_*` and a `discord_*` column +group until PR 4; a module cannot `ALTER` a core table, so a registered leg had nowhere to live. The +per-leg state moved to `announce_job_legs (job_id, leg, status, attempts, last_error, +next_attempt_at)` and `leg` is a stored value. The parent `status` rollup is over *all* the job's +legs — done when every leg delivered, failed when every leg gave up, partial in between; and `done` +when a job has no legs at all, since nothing is left to deliver. **`onBoot(fn)` / `onShutdown(fn)`** — §2.5. @@ -665,6 +701,54 @@ files. Two consequences: Neither changes a decision; both are corrected here rather than left to be tripped over when the extraction is counted against the plan. +### 6.5 Grandfathered names, and why the prefix rules survive them + +§2.4 requires a module's stream ids and announce legs to carry its module id. Eight names predate the +module system and cannot take it: + +| Kind | Names | Why they cannot be renamed | +| --- | --- | --- | +| Streams | `server.status`, `idoc.warning`, `champ.start`, `governor.election`, `vendor.sale`, `house.idoc`, `account.login` | stored in `notification_subs` rows; read by a shipped Android client | +| Announce leg | `towncrier` | a stored value in `announce_job_legs.leg` and the body of the retry endpoint | + +They are allowed to **`uo` alone**, by an explicit per-module allowlist — the same shape and the same +reasoning as the loader's `LEGACY_TABLE_PREFIXES` for module-uo's 27 tables. Grandfathering by +allowlist rather than dropping the rule is what keeps the rule real for every module written after +this one; the alternative leaves the first name collision to be discovered by a module silently +adopting someone else's stream. + +### 6.6 An extension slot is invisible to static analysis — core needs the merge too + +§6.1 settled the fragment merge for *modules*. PR 4 found that core needs the identical machinery for +its **own** slot fills, one phase earlier than the plan expected. + +A slot's router is created by `registries.declareSlot()` and filled later, so there is no literal +`use(require(...))` for swagger-autogen to follow. Moving the six `/admin/users/:id/shard/*` routes +behind `admin.users.detail` therefore deleted 407 lines from `swagger-output.json` — with +`Swagger-autogen: Success` and no warning. Same failure as §7.4, different cause, and it would have +shipped six undocumented core routes against CLAUDE.md's standing rule. + +`npm run swagger` now has a second step (`swagger/slotSpecs.js`): for each **filled** slot, generate a +fragment by pointing swagger-autogen at that router's own file, re-root its paths at the prefix the +router actually hangs at, and merge. Two things are derived rather than written down, because a +written-down copy drifts: + +- **which** slots — from `registries.filledSlots()`; +- **where** each hangs — by finding the slot's own router object in the live express stack, decoding + the mount prefixes above it with `scripts/routeManifest.js`'s own `mountPath`, so the manifest and + the spec can never disagree about what a mount decodes to. + +An empty fragment is a hard build failure, because an empty fragment is exactly what the silent drop +looks like. The merge itself is `swagger/mergeSpec.js` — the ~40-line helper §6.1a already owed core +for module fragments, written here and proved against core's own slot before a module depends on it. +This is **build**-time and lands in the committed spec, because slot routes are core's; a module's +fragment is still merged at **request** time into `/api/docs.json` (§6.1a), and `swagger-output.json` +stays reproducible on any machine regardless of what is installed. + +`registerExtension` therefore takes a third, **core-only** argument: the file its router is generated +from. A module needs no equivalent — it ships a prebuilt `swagger-fragment.json`, because core never +has its sources to analyse. + --- ## Part 7 — What the spike proved diff --git a/website/MODULE_SYSTEM.md b/website/MODULE_SYSTEM.md index 33d7194..92f99bf 100644 --- a/website/MODULE_SYSTEM.md +++ b/website/MODULE_SYSTEM.md @@ -153,16 +153,36 @@ Everything else is a folder move. These are not: 1. **`src/config/notificationStreams.js`** — the push stream catalog. `mapShardEvent()` and most of `STREAMS` are shard-derived, and it imports `PUBLIC_KINDS` from `utils/shardBroadcast`. Push *infrastructure* is core; this *catalog* is module content. - → `registerNotificationStreams({ streams, mapEvent })`. + → `registerNotificationStreams(streams)`. 2. **`src/utils/pushDispatch.js`** — core infrastructure, but `fromShardEvent()` (line 112) requires the `shardLinks` model (line 21) and `mapShardEvent` (line 23). → invert: `publish()` stays core, `fromShardEvent` moves into the module and calls it. 3. **`src/utils/announceWorker.js`** — the news dispatcher, with two delivery legs: Discord (core) and town crier (module, via `uoLinkClient.postTownCrier`, line 36). - → `registerAnnounceLeg({ leg, dispatch, classify })`. + → `registerAnnounceLeg({ leg, label, dispatch, classify })`. `src/utils/newsGump.js` is module-side (news → in-game gump) and moves whole. +**Done in Phase 2 PR 4**, with core still the only registrant — the registries are +`src/modules/registries.js` and core goes through them by the same door a module will +(`registerCore()`, called explicitly from `app.js` before `modules.load()`). What each of the three +became: + +1. Split in two. `config/coreStreams.js` is core's one stream (`news.post`, produced by the website's + own posts path); `config/shardStreams.js` is the other seven plus `mapShardEvent` and the + public-safety filter, and moves to module-uo whole. `registerNotificationStreams` lost its + `mapEvent` half — see [`MODULE_API.md`](MODULE_API.md) §2.4 for why that was a leftover, and what + follows for the public/personal split. +2. Inverted. `pushDispatch.js` is `publish` + `isAllowedEndpoint` and nothing else; + `utils/shardPush.js` holds `fromShardEvent` and is what `shardIngest` now calls. +3. Legs became registrations, and per-leg **rows**. The `towncrier_*` / `discord_*` column groups on + `announce_jobs` could never have held a module's leg — a module cannot `ALTER` a core table — so + they became `announce_job_legs`, backfilled and dropped in the same idempotent replay. The worker + no longer contains the word "towncrier": it iterates whatever is registered. + +The residue in core is a one-time backfill block in `schema.sql`, deletable once every deployment has +booted it, and the two lines of `registerCore()` that Phase 3 turns into module-uo's `register()`. + ### 1.9 A fourth mount shape: module routes under a core resource `router/v1/admin/users.router.js` mounts `usersShard.controller.js` at six UO sub-paths of a **core** @@ -175,6 +195,17 @@ narrow **extension slot** on `/admin/users/:id` that the module mounts into, so what "shard" means and all six URLs are preserved. Only core may declare an extension slot; a module may not invent one. +**Both done in Phase 2 PR 4**, with core filling its own slot: the six paths are +`router/v1/admin/usersShard.router.js`, registered into `admin.users.detail` by `registerCore()`, and +Phase 3 changes the registrant rather than the routes. The slot's router is created at declare time +and filled later, because `users.router.js` is required while `app.js` is still being built. It is +mounted **last** on the resource, so core wins any path conflict by first-match. + +One consequence was not foreseen and is worth the warning: **a slot is invisible to static analysis.** +There is no literal mount for `swagger-autogen` to follow, so the move silently deleted all six paths +from `swagger-output.json` while printing `Success`. The OpenAPI build now merges a generated +fragment per filled slot — [`MODULE_API.md`](MODULE_API.md) §6.6. + ### 1.10 The Discord bot has no UO logic The draft listed the bot's "UO-specific event/moderation logic" as an extraction candidate. Grepping @@ -453,7 +484,7 @@ too (API §7.2). Exit criterion: `routes.manifest.json` diff is zero lines and every existing test passes. If Phase 2 changes one URL, it is wrong. -**Progress: PRs 1-3 done.** +**Progress: PRs 1-4 done.** - **PR 1** — `installed_modules` and the state machine, with the stored shape and the boot rules settled in §2.4 above. @@ -476,8 +507,31 @@ changes one URL, it is wrong. as, because the file is replayed on **every boot**. Found while wiring it: `npm run seed` calls `ensureSchema()` without ever requiring `app.js`, so the replay has to tolerate an unscanned loader. +- **PR 4** — the three de-entanglement registries, `src/modules/registries.js`. Core's own streams, + its Discord announce leg and its users-detail routes all go through them, so the seams are + exercised on every boot before a module depends on them; §1.8 and §1.9 above record what each + became. Four decisions landed with it, all recorded in [`MODULE_API.md`](MODULE_API.md): announce + legs became a **child table** rather than waiting for Phase 3 (§2.4 — a module cannot alter a core + table, so a registered leg had nowhere to live); **`mapEvent` dropped** from the stream registry + (§2.4 — a leftover from before the push inversion was settled); **core registers through the same + staging area a module uses**; and core's six shard sub-paths **moved behind the slot now** rather + than in Phase 3. + + Registering is **validate-then-commit**: the loader stages a module's claims and the second pass + commits them, so a module that throws halfway through `register()` — or fails a later validation + step — leaves nothing behind. That is the registry-side twin of PR 2's second-pass mount rule. + + Two build tools needed teaching, both because a mechanism this PR introduced is one they had never + seen. `scripts/routeManifest.js` could not decode a **parameterised mount**: its unwinder expected + a group shape express does not emit, and the branch had never run. It threw rather than guessing, + which is exactly what it is for. And `swagger-autogen` could not follow a route into an extension + **slot**, deleting 407 lines while reporting success; the fix is the fragment merge core owed + anyway ([`MODULE_API.md`](MODULE_API.md) §6.6). + There is still no module on the volume and no boot wiring, so this changes nothing an operator or a -client can see: 856 tests pass and `routes.manifest.json` is unchanged at 229 routes. +client can see: **884 tests pass** and `routes.manifest.json` is unchanged at 229 routes. The two +lines of OpenAPI that do move are the retry endpoint's summary and its `leg`, which is no longer a +fixed enum because the leg set is whatever has been registered. **Phase 3 — Extract `module-uo`.** Moves out of `website/`: the 8 model directories and their 25 tables; the nine UO `utils/` files plus `newsGump.js`; the 13 router/controller files; diff --git a/website/website-README.md b/website/website-README.md index 527e558..31c7fe1 100644 --- a/website/website-README.md +++ b/website/website-README.md @@ -482,7 +482,7 @@ Copy `.env.example` (Compose) or `server/.env.example` (local) and fill in. **`. | `CLIENT_ORIGIN` | `http://localhost:5173` | enables CORS in dev only | | `LOG_LEVEL` / `FILE_LOG_LEVEL` | `info` / `debug` | console / file verbosity | | `LOG_TO_FILE` / `LOG_DIR` / `LOG_FILE` | `true` / `/logs` / `app.log` | log file (bind-mounted to `./logs` in Docker) | -| `ANNOUNCE_POLL_MS` | `15000` | how often the news-announcement dispatcher sweeps `announce_jobs` for due/retry legs (town crier + Discord) | +| `ANNOUNCE_POLL_MS` | `15000` | how often the news-announcement dispatcher sweeps `announce_job_legs` for due/retry legs (whichever are registered — Discord is core's, the town crier is module-uo's) | | `TOWNCRIER_DURATION_SEC` | `3600` | how long a news post's in-game town-crier message stays up (≤ `86400`) | ---