diff --git a/website/MODULE_API.md b/website/MODULE_API.md index f350220..72aa5a9 100644 --- a/website/MODULE_API.md +++ b/website/MODULE_API.md @@ -539,20 +539,82 @@ registry.registerNav('uo', { `group` names an existing core group; an unknown group name appends a new group at the end rather than dropping the item. `order` sorts within the group, core items keeping their current positions. -`feature` (public area only) names a flag resolved by §3.3's provider. +`feature` names a flag resolved by the provider below. -`MOD_PATHS` in `AdminLayout.jsx:109` — today a hardcoded allowlist of two UO paths — becomes a -computation over each item's `roles`, so moderator visibility follows from the registration instead -of from a second list that has to be kept in sync. +Six details settled when this was built (Phase 2 PR 8, `client/src/modules/nav.js`): + +- **`order` on a flat nav is a position among core's rows**, which are keyed by their index; an + explicit `order` beats a core row that merely sits at that index. A row with **no** `order` + appends after the coded rows rather than defaulting to 0 — otherwise "I didn't ask for a + position" would mean "put me first", which is the one place a module could take over the header + without asking for anything. +- **A row with no `group` on the admin sidebar gets a trailing untitled group of its own**, not a + place in one of core's untitled groups. Those are Dashboard at the top and Account at the bottom; + a module page belongs beside neither, and core does not invent a display title out of a module id. +- **A group a module created is itself a legal override destination.** It falls out of building the + destination set from the merged base nav, and is recorded so it is not mistaken for an accident. +- **A row whose `to` collides with an existing row is dropped, with a console warning.** `to` is the + key the override layer stores under and React renders by, so two rows sharing one would give an + admin a single editor row that silently moves both. Core's row wins, since that is the one any + stored override was written against. +- **`feature` is not public-area-only.** An earlier draft of this section said it was, on the + grounds that core's admin and player navs carry no flags. They still do not — but a module row + that declares a gate and has it silently ignored is a trap, so the gate is applied in all three + areas and the field means one thing everywhere. +- **The interleave happens before the override merge, and that ordering is load-bearing.** The merge + is keyed by `to` and drops any key its base array does not declare, so module rows appended + afterwards would be unorderable, unrelabellable and unhideable — a visible regression for every + operator who has ever edited a nav, the day the UO rows leave core. + +Moderator confinement, `MOD_PATHS` in `AdminLayout.jsx:109` — a hardcoded allowlist of five paths — +becomes a computation over each item's `roles`, so moderator visibility follows from the +registration instead of from a second list that has to be kept in sync. It moved to +`client/src/lib/adminNav.js`, plain JS so the test runner can reach it, along with the redirect that +confines a moderator who deep-links. **The redirect derives from the base nav, never the +override-merged one**: an override is presentation and must not move an authorization boundary in +either direction — hiding a row must not also bar someone from the page, and un-hiding one must not +admit them to it. + +Deriving it changed what a moderator sees, in both cases toward what the server already permitted: +**Dashboard**, whose `roles` had always named moderator while `MOD_PATHS` omitted it, and **My +Characters**, which is ungated self-service. It also fixed a defect the two lists had between them — +`/admin/houses` was on the sidebar and not in the redirect's own third list, so a moderator clicking +Houses in their own nav was bounced back to Moderation. The pipeline is unchanged from `THEMING_AND_NAV.md`, with one new first step: -> registered defaults (core **+ modules**) → role/feature filtering → admin overrides → rendered nav +> registered defaults (core **+ modules**) → admin overrides → role/feature filtering → rendered nav + +An earlier draft of this line had the last two the other way round. The filter runs **last** and +that is deliberate — it is what keeps it a boundary an override cannot cross (`THEMING_AND_NAV.md` +§7), and both layouts have always been written that way. **`registerFeatureProvider`** — core keeps a generic flag context; the module supplies the hook that fills its namespace (`useShardFeatures` for `uo`). With no module installed the filter is a correct no-op, because no core nav item carries a `feature` today. +**The namespace comes from the registration, not from the string.** A row's `feature` is resolved by +the provider its own module registered, so a module author writes `feature: 'status'` exactly as it +reads today: nothing parses a prefix, and a typo'd namespace is not a thing that can exist. Core's +own rows carry no `moduleId` and resolve against the owner id **`core`** — which is what core +registers `useShardFlags` under (`main.jsx`), the client twin of the server's `registries.registerCore()`. +So the ten shard-gated rows in the public header already run through the module seam rather than +beside it, and Phase 3 deletes core's registration instead of rewriting the header. + +A provider hook returns **a Set-like of the flags this viewer may see, or `null`** while the answer +is in flight. Every unknown — no provider, a null answer, a provider that returned something without +a `has`, a malformed row — **shows the link**. This is presentation and the server is the gate, so a +UI mistake that hides a page from someone entitled to it is worse in every case than one that shows +a link which then 403s. + +Core calls every registered provider's hook unconditionally, in a fixed order, at the top of the +context component. That is legal because the rules of hooks require the same hooks in the same order +on every render of a component, not a statically known list: registration completes before the first +render (§3.1), nothing unregisters, and the provider list is snapshotted per component instance +anyway. `registry.featureProviders()` is a module export and deliberately **not** a member of the +`registry` object handed to modules — a module asks for a namespace it knows the name of, and has no +business enumerating what everyone else registered. + ### 3.4 `ui` — the shared component kit **This is the largest addition Phase 1 makes to the plan, and it is not optional** (§6.2; approved diff --git a/website/MODULE_SYSTEM.md b/website/MODULE_SYSTEM.md index 69270fa..82be038 100644 --- a/website/MODULE_SYSTEM.md +++ b/website/MODULE_SYSTEM.md @@ -97,6 +97,14 @@ moderator-visible. **Resolved:** nav registration takes a target group and order (`{ group: 'Moderation', order: 30 }`), and `MOD_PATHS` becomes a `roles`-derived computation rather than a path allowlist. +Built in Phase 2 PR 8. Two things it turned up that this section did not predict. The interleave has +to happen **before** the admin-override merge and not after it, because that merge drops any `to` +its base array does not declare — appending module rows afterwards would leave them uneditable in +Admin → Navigation, which today's UO rows are not. And there was a **third** hardcoded list: the +redirect that confines a moderator checked three path prefixes, while `MOD_PATHS` listed five paths, +and they disagreed about `/admin/houses` — a moderator who clicked Houses in their own sidebar was +bounced straight back to Moderation. One derivation cannot disagree with itself. + ### 1.5 The public nav's feature-gating mechanism is itself a shard system Ten of the sixteen entries in `SiteHeader.jsx`'s NAV carry a `feature:` key (`status`, `champs`, @@ -108,6 +116,12 @@ Extracting the module removes the provider that core's own nav filter depends on registers its `useShardFeatures` for its own namespace. No core nav item carries a `feature` today, so with no module installed the filter is a correct no-op. +Built in Phase 2 PR 8, and core registers into it **now** rather than at extraction: `useShardFlags` +goes in under the owner id `core`, so the ten rows above are already resolved through the seam and +`SiteHeader` runs one mechanism instead of two. Which provider answers a row is decided by the +module that registered it, not by a prefix parsed out of the flag name, so those ten keep the exact +strings they carry today and Phase 3 moves them without a rename. + ### 1.6 There is no migration system to model a module migration runner on `server/db/schema.sql` is a single idempotent file — 1,380 lines, 67 tables — replayed in full on @@ -627,6 +641,43 @@ fixed enum because the leg set is whatever has been registered. filesystem-conditional static mount, not API surface, for the same reason `/uploads` and `/brand` are not in the manifest. +- **PR 8** — the nav half PR 7 deferred, and the two seams §1.4 and §1.5 asked for: `withModuleNav` + (`client/src/modules/nav.js`) interleaving module rows into core's three navs, `MOD_PATHS` and the + moderator redirect replaced by a `roles`-derived computation in `client/src/lib/adminNav.js`, and + the generic feature-provider seam (`modules/features.jsx` + `modules/featureGate.js`) that core + registers its own `useShardFlags` into. Four decisions, all recorded in + [`MODULE_API.md`](MODULE_API.md) §3.3. + + **The interleave happens before the admin-override merge**, which is the decision the rest follow + from: the merge is keyed by `to` and drops any key its base array does not declare, so module rows + appended after it would be unorderable, unrelabellable and unhideable — and today's UO rows are + all three of those things, so appending would make the extraction a visible regression for every + operator who has ever edited their nav. Doing it first means a module row is an ordinary row to + everything downstream: nothing in `navOverrides.js`, `NavEditor.jsx` or the layouts knows a module + exists. **Moderator visibility derives purely from `roles`**, which moves two rows the old + allowlist withheld — Dashboard, whose `roles` had always named moderator, and My Characters, which + is ungated self-service — both toward what the server already permitted. **A row's `feature` is + resolved by the provider its own module registered**, so the namespace comes from the registration + rather than from a parsed string prefix. And **core registers through the same seam**, under the + owner id `core`, so `SiteHeader` holds one mechanism instead of two and Phase 3 is a deletion. + + The PR also fixed a defect that predates the module system: the moderator redirect was a **third** + hardcoded list, and it disagreed with `MOD_PATHS` about `/admin/houses`, so a moderator who + clicked Houses in their own sidebar was bounced back to Moderation. The derived allow-list is + computed from the **base** nav rather than the merged one, so an override — which is presentation + — cannot move that boundary in either direction. + + **933 server tests** (unchanged — this PR is client-only) and **160 client tests** (+37) pass; + `routes.manifest.json` is unchanged at 230 routes and the OpenAPI spec regenerates byte-identical. + The [§7.7](MODULE_API.md#77-the-client-half-has-to-be-verified-in-a-browser--the-timing-bug-no-test-could-see) + browser smoke was re-run, since this is the seam that rule exists for: a throwaway module + registering nav in all three areas and a provider granting one flag and withholding another. It + confirmed, in Chrome with the console open, that the row lands inside core's Moderation group + rather than in an appended block, that the withheld row does not render while the granted one + does, that a moderator reaches both `/admin/houses` and the module's own admin page, and that an + admin can relabel a module row in Admin → Navigation and have it persist and apply — the whole + point of merging before the override layer. Zero CSP reports, zero console errors. + **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; `scripts/importSpawnAtlas.js` and `db/spawnAtlas.art.json`; `usersShard.controller.js` **minus diff --git a/website/THEMING_AND_NAV.md b/website/THEMING_AND_NAV.md index 3611d54..d7df39d 100644 --- a/website/THEMING_AND_NAV.md +++ b/website/THEMING_AND_NAV.md @@ -536,10 +536,20 @@ Three existing behaviors the merge must not disturb: no menu. `pruneNav` applies the gate inside a section and then drops one it leaves empty. -- **Moderator confinement.** `AdminLayout` restricts moderators to `MOD_PATHS` and - redirects them out of anything else. Overrides apply before that filter, so a - moderator can still end up with a legitimately short sidebar — but the redirect - effect must keep working untouched. +- **Moderator confinement.** `AdminLayout` restricts moderators to the rows their + role carries and redirects them out of anything else. Overrides apply before + that filter, so a moderator can still end up with a legitimately short sidebar + — but the redirect effect must keep working untouched. + + Amended by the module system's Phase 2 PR 8: this used to be a hardcoded + `MOD_PATHS` allowlist plus a second, differently-worded prefix check in the + redirect, and the two had drifted — `/admin/houses` was on the sidebar and not + in the redirect, so a moderator who clicked Houses was bounced to Moderation. + Both are now derived from each row's own `roles` + ([`adminNav.js`](../../website/client/src/lib/adminNav.js)), and the redirect + derives from the **base** nav rather than the merged one, which is what keeps + an override from moving the boundary in either direction. See + [`MODULE_SYSTEM.md`](MODULE_SYSTEM.md) §1.4. - **Empty groups.** `AdminLayout` drops groups whose items all filtered out. An override that hides every item in a group must produce no orphaned header.