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
+Six details settled when this was built (Phase 2 PR 8, `client/src/modules/nav.js`):
-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.
+- **`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

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

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
+- **Moderator confinement.** `AdminLayout` restricts moderators to the rows their
-  redirects them out of anything else. Overrides apply before that filter, so a
+  role carries and redirects them out of anything else. Overrides apply before
-  moderator can still end up with a legitimately short sidebar — but the redirect
+  that filter, so a moderator can still end up with a legitimately short sidebar
-  effect must keep working untouched.
+  — 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.