feat(modules): interleave module nav, derive moderator confinement
All checks were successful
PR Checks / bot-install (pull_request) Successful in 17s
PR Checks / server-tests (pull_request) Successful in 1m34s
PR Checks / client-build (pull_request) Successful in 8m58s

Phase 2, PR 8 of docs/website/MODULE_SYSTEM.md 2.7 - the nav half PR 7
deferred, plus the two seams 1.4 and 1.5 asked for.

withModuleNav (client/src/modules/nav.js) merges an installed module's rows
into core's three navs BEFORE the admin-override merge, and that ordering is
the design. applyNavOverrides and buildPublicNav are keyed by `to` and drop
any key their base array does not declare, so rows appended after the merge
would be unorderable, unrelabellable and unhideable in Admin - Navigation.
Today's UO rows are all three of those things, so appending would make the
extraction a visible regression for anyone who has ever edited their nav.
Merging first means a module row is an ordinary row downstream: nothing in
navOverrides.js, NavEditor.jsx or the layouts knows a module exists.

MOD_PATHS is gone. Moderator visibility and the redirect that confines a
moderator both derive from each row's own `roles`, in the new plain-JS
lib/adminNav.js (plain so the DOM-less runner can reach it). Two rows move,
both toward what the server already permitted: Dashboard, whose roles had
always named moderator, and My Characters, which is ungated self-service.

That also fixes a defect predating the module system. The redirect was a
THIRD hardcoded list - three path prefixes against MOD_PATHS' five paths -
and they disagreed 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, never the override-merged one: an override is
presentation and must not move an authorization boundary either way.

The feature seam (modules/features.jsx + modules/featureGate.js) resolves a
row's `feature` against the provider its OWN module registered, so the
namespace comes from the registration and no string carries a parsed prefix.
Core registers useShardFlags under the owner id `core` - the client twin of
registries.registerCore() - so the ten shard-gated header rows already run
through the seam and Phase 3 deletes a registration instead of rewriting
SiteHeader. Every unknown fails open: no provider, a null answer while a
fetch is in flight, or a junk return all show the link, because the server is
the gate and hiding a page from someone entitled to it is the worse mistake.

933 server tests (unchanged - this PR is client-only), 160 client tests
(+37). routes.manifest.json unchanged at 230 routes; the OpenAPI spec
regenerates byte-identical.

Re-ran the MODULE_API.md 7.7 browser smoke, 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: the row lands inside
core's Moderation group rather than an appended block, the withheld row does
not render, a moderator reaches both /admin/houses and the module's admin
page, and an admin can relabel a module row and have it persist and apply.
Zero CSP reports, zero console errors.

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
2026-08-10 23:42:02 -05:00
parent e3c999b704
commit a45a3d120a
17 changed files with 1119 additions and 228 deletions

View File

@@ -0,0 +1,125 @@
import { test } from 'node:test'
import assert from 'node:assert/strict'
import { navItemVisibleTo, allowedPathsFor, isAllowedPath } from '../src/lib/adminNav.js'
// Moderator confinement, derived from each row's `roles` (Phase 2 PR 8 —
// MODULE_SYSTEM.md §1.4). This replaced two hardcoded path lists that had
// drifted apart from each other, so the tests worth having are the ones that
// pin what a moderator may now see and reach, and the shape of the match.
// The real sidebar, trimmed to the rows that decide something here.
const NAV = [
{ items: [{ to: '/admin', label: 'Dashboard', end: true, roles: ['admin', 'editor', 'moderator'] }] },
{
title: 'Moderation',
items: [
{ to: '/admin/moderation', label: 'Moderation', roles: ['admin', 'moderator'] },
{ to: '/admin/moderation/appeals', label: 'Appeals', roles: ['admin', 'moderator'] },
{ to: '/admin/shard-ops', label: 'In-Game Ops', roles: ['admin', 'moderator'] },
{ to: '/admin/houses', label: 'Houses', roles: ['admin', 'moderator'] },
],
},
{
title: 'System',
items: [
{ to: '/admin/users', label: 'Users', roles: ['admin'] },
{ to: '/admin/settings', label: 'Settings', roles: ['admin'] },
],
},
{
items: [
{ to: '/admin/characters', label: 'My Characters' },
{ to: '/admin/account', label: 'Account' },
],
},
]
const visibleTo = (role) =>
NAV.flatMap((g) => g.items)
.filter((i) => navItemVisibleTo(i, role))
.map((i) => i.to)
test('a row with no roles is visible to every staff role', () => {
// Self-service: staff are a superset of players, so a moderator reaching their
// own characters is not a privilege, it is the thing every account has.
for (const role of ['admin', 'editor', 'moderator']) {
assert.equal(navItemVisibleTo({ to: '/admin/account' }, role), true)
}
})
test('a role not named on the row cannot see it', () => {
assert.equal(navItemVisibleTo({ to: '/admin/users', roles: ['admin'] }, 'moderator'), false)
assert.equal(navItemVisibleTo({ to: '/admin/users', roles: ['admin'] }, 'admin'), true)
// An unknown or absent role sees only the ungated rows.
assert.equal(navItemVisibleTo({ to: '/admin/users', roles: ['admin'] }, undefined), false)
assert.equal(navItemVisibleTo({ to: '/admin/account' }, undefined), true)
})
test('what a moderator sees is exactly the moderation section, plus self-service', () => {
// The two additions the derivation makes over the old MOD_PATHS list are
// Dashboard — whose roles have always named moderator, so the two lists
// disagreed — and My Characters. Both are already permitted server-side.
assert.deepEqual(visibleTo('moderator'), [
'/admin',
'/admin/moderation',
'/admin/moderation/appeals',
'/admin/shard-ops',
'/admin/houses',
'/admin/characters',
'/admin/account',
])
})
test('an admin still sees everything and an editor still sees nothing extra', () => {
assert.equal(visibleTo('admin').length, NAV.flatMap((g) => g.items).length)
assert.deepEqual(visibleTo('editor'), ['/admin', '/admin/characters', '/admin/account'])
})
test('a row with `end` matches exactly — the dashboard is not a prefix', () => {
// The bug this shape exists to prevent: treating `/admin` as a prefix would
// make every path in the admin area allowed for anyone who can see Dashboard.
const allowed = allowedPathsFor(NAV, 'moderator')
assert.equal(isAllowedPath('/admin', allowed), true)
assert.equal(isAllowedPath('/admin/users', allowed), false)
assert.equal(isAllowedPath('/admin/users/12', allowed), false)
})
test('every other row covers its own sub-routes', () => {
const allowed = allowedPathsFor(NAV, 'moderator')
assert.equal(isAllowedPath('/admin/moderation/appeals/12', allowed), true)
assert.equal(isAllowedPath('/admin/characters/0x4001', allowed), true)
})
test('a sibling path that merely shares a prefix is NOT covered', () => {
const allowed = allowedPathsFor(NAV, 'moderator')
// `/admin/houses-secret` starts with `/admin/houses` as a string; the match is
// on path segments, so it does not start with `/admin/houses/`.
assert.equal(isAllowedPath('/admin/houses-secret', allowed), false)
assert.equal(isAllowedPath('/admin/houses/42', allowed), true)
})
test('Houses is reachable, which is the defect the derivation fixed', () => {
// The redirect used to allow only /admin/moderation*, /admin/shard-ops* and
// /admin/account, while the sidebar showed Houses — so a moderator clicking a
// row in their own nav was bounced back to Moderation.
const allowed = allowedPathsFor(NAV, 'moderator')
assert.equal(isAllowedPath('/admin/houses', allowed), true)
})
test('a module row a moderator may see is reachable without core listing it', () => {
// The reason this is derived at all: core cannot hardcode a path it has never
// heard of, and a module row arrives with `roles` like any other.
const withModule = [
...NAV,
{ title: 'Shard', items: [{ to: '/admin/uo/shard-ops', label: 'Ops', roles: ['admin', 'moderator'], moduleId: 'uo' }] },
]
const allowed = allowedPathsFor(withModule, 'moderator')
assert.equal(isAllowedPath('/admin/uo/shard-ops', allowed), true)
assert.equal(isAllowedPath('/admin/uo/shard-ops/queue', allowed), true)
})
test('a nav that is not there does not throw', () => {
assert.deepEqual(allowedPathsFor(null, 'moderator'), [])
assert.equal(isAllowedPath('/admin', undefined), false)
})

View File

@@ -0,0 +1,85 @@
import { test } from 'node:test'
import assert from 'node:assert/strict'
import { buildFeatureGate, OPEN_GATE } from '../src/modules/featureGate.js'
// The feature seam's decision logic (MODULE_SYSTEM.md §1.5, MODULE_API.md §3.3).
// Every branch here fails OPEN, and that is the property under test as much as
// the happy path: this is presentation, the server is the gate, and 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.
const flags = (...names) => new Set(names)
test('a row with no feature is always visible', () => {
const gate = buildFeatureGate(new Map([['uo', flags()]]))
assert.equal(gate({ to: '/site/news' }), true)
})
test('a core row resolves against the owner id `core`', () => {
// Core's ten shard-gated rows carry no moduleId, and core registers its
// provider under `core` (main.jsx) precisely so they resolve without one.
const gate = buildFeatureGate(new Map([['core', flags('atlas')]]))
assert.equal(gate({ to: '/site/atlas', feature: 'atlas' }), true)
assert.equal(gate({ to: '/site/market', feature: 'market' }), false)
})
test('a module row resolves against ITS module, not another one', () => {
const gate = buildFeatureGate(
new Map([
['uo', flags('atlas')],
['rust', flags('market')],
]),
)
assert.equal(gate({ to: '/uo/atlas', feature: 'atlas', moduleId: 'uo' }), true)
// `market` is a flag the OTHER module grants. Resolution is by registration,
// so there is no string a module can write to borrow it.
assert.equal(gate({ to: '/uo/market', feature: 'market', moduleId: 'uo' }), false)
assert.equal(gate({ to: '/rust/market', feature: 'market', moduleId: 'rust' }), true)
})
test('no provider for the owner shows the row', () => {
// The no-module-installed case, and the reason the filter is a correct no-op
// on a bare core rather than a nav that renders nothing.
const gate = buildFeatureGate(new Map())
assert.equal(gate({ to: '/site/atlas', feature: 'atlas' }), true)
assert.equal(gate({ to: '/uo/atlas', feature: 'atlas', moduleId: 'uo' }), true)
})
test('a provider still loading shows the row', () => {
// useShardFlags returns null until its fetch lands. Blanking the nav on every
// page load and filling it in a moment later is the behaviour this avoids.
const gate = buildFeatureGate(new Map([['core', null]]))
assert.equal(gate({ to: '/site/atlas', feature: 'atlas' }), true)
})
test('a provider that returned something unusable shows the row', () => {
for (const bad of [undefined, 42, 'atlas', {}, []]) {
const gate = buildFeatureGate(new Map([['core', bad]]))
assert.equal(gate({ to: '/site/atlas', feature: 'atlas' }), true, `failed closed on ${JSON.stringify(bad)}`)
}
})
test('an array-backed provider is not silently treated as a Set', () => {
// `[].has` does not exist, so this is the unusable case above rather than a
// membership test that quietly always fails. Asserted so that a future
// "helpful" normalisation knows it changed a documented behaviour.
const gate = buildFeatureGate(new Map([['core', ['atlas']]]))
assert.equal(gate({ to: '/site/atlas', feature: 'atlas' }), true)
})
test('a missing map, or a junk row, shows rather than throws', () => {
assert.equal(buildFeatureGate(null)({ feature: 'atlas' }), true)
assert.equal(buildFeatureGate(new Map())(null), true)
assert.equal(buildFeatureGate(new Map())(undefined), true)
})
test('any Set-like satisfies a provider — core does not require a Set', () => {
const gate = buildFeatureGate(new Map([['uo', { has: (name) => name === 'ruleset' }]]))
assert.equal(gate({ feature: 'ruleset', moduleId: 'uo' }), true)
assert.equal(gate({ feature: 'champs', moduleId: 'uo' }), false)
})
test('the open gate is what a component outside the provider gets', () => {
assert.equal(OPEN_GATE({ feature: 'anything' }), true)
})

View File

@@ -0,0 +1,187 @@
import { test, beforeEach } from 'node:test'
import assert from 'node:assert/strict'
import { withModuleNav } from '../src/modules/nav.js'
import { registerNav, _reset } from '../src/modules/registry.js'
import { applyNavOverrides, buildPublicNav } from '../src/lib/navOverrides.js'
// The interleave of module nav rows into core's nav (MODULE_API.md §3.3, Phase 2
// PR 8). Tested against the real merge next door rather than in isolation,
// because the property that matters is a relationship between the two: a module
// row has to be indistinguishable from a core row to everything downstream, and
// the way to prove that is to run the downstream thing on it.
const PUBLIC = [
{ label: 'Home', to: '/', end: true },
{ label: 'News', to: '/site/news' },
{ label: 'About', to: '/site/about' },
]
const ADMIN = [
{ items: [{ to: '/admin', label: 'Dashboard', end: true, roles: ['admin', 'moderator'] }] },
{ title: 'Moderation', items: [{ to: '/admin/moderation', label: 'Moderation' }] },
{ title: 'System', items: [{ to: '/admin/users', label: 'Users' }, { to: '/admin/settings', label: 'Settings' }] },
{ items: [{ to: '/admin/account', label: 'Account' }] },
]
beforeEach(() => _reset())
test('with no module installed the base array is returned unchanged', () => {
// Identity, not a copy: this is what makes the useMemo in each layout honest,
// and what guarantees an instance with no modules renders what it renders now.
assert.equal(withModuleNav(PUBLIC, 'public'), PUBLIC)
assert.equal(withModuleNav(ADMIN, 'admin'), ADMIN)
})
test('a flat nav places a module row by the order it asked for', () => {
registerNav('uo', { area: 'public', items: [{ label: 'Atlas', to: '/uo/atlas', order: 1 }] })
assert.deepEqual(
withModuleNav(PUBLIC, 'public').map((i) => i.label),
['Home', 'Atlas', 'News', 'About'],
)
})
test('a flat row with no order appends rather than jumping to the front', () => {
// The 0-default trap: `order ?? 0` would put an unordered row first, which is
// the one place a module could take over the nav without asking for anything.
registerNav('uo', { area: 'public', items: [{ label: 'Atlas', to: '/uo/atlas' }] })
assert.deepEqual(
withModuleNav(PUBLIC, 'public').map((i) => i.label),
['Home', 'News', 'About', 'Atlas'],
)
})
test('an explicit order beats a core row that merely sits at that index', () => {
registerNav('uo', { area: 'public', items: [{ label: 'Atlas', to: '/uo/atlas', order: 2 }] })
const labels = withModuleNav(PUBLIC, 'public').map((i) => i.label)
assert.deepEqual(labels, ['Home', 'News', 'Atlas', 'About'])
})
test('an admin row lands INSIDE the core group it names', () => {
registerNav('uo', {
area: 'admin',
items: [
{ label: 'In-Game Ops', to: '/admin/uo/shard-ops', group: 'Moderation', order: 30 },
{ label: 'Shard', to: '/admin/uo/link', group: 'System', order: 0 },
],
})
const nav = withModuleNav(ADMIN, 'admin')
assert.deepEqual(nav.map((g) => g.title), [undefined, 'Moderation', 'System', undefined])
assert.deepEqual(nav[1].items.map((i) => i.label), ['Moderation', 'In-Game Ops'])
// order 0 puts it above both core rows, which is the whole point of the field.
assert.deepEqual(nav[2].items.map((i) => i.label), ['Shard', 'Users', 'Settings'])
})
test('an unknown group appends a new group instead of dropping the row', () => {
// A typo must cost a position, never a link.
registerNav('uo', { area: 'admin', items: [{ label: 'Atlas', to: '/admin/uo/atlas', group: 'Moderaton' }] })
const nav = withModuleNav(ADMIN, 'admin')
assert.equal(nav.length, ADMIN.length + 1)
assert.deepEqual(nav.at(-1), { title: 'Moderaton', items: [{ label: 'Atlas', to: '/admin/uo/atlas', group: 'Moderaton', moduleId: 'uo' }] })
})
test('an admin row with no group gets a trailing untitled group of its own', () => {
// NOT folded into one of core's untitled groups: those are Dashboard at the
// top and Account at the bottom, and a module page belongs beside neither.
registerNav('uo', { area: 'admin', items: [{ label: 'Atlas', to: '/admin/uo/atlas' }] })
const nav = withModuleNav(ADMIN, 'admin')
assert.equal(nav.length, ADMIN.length + 1)
assert.equal(nav.at(-1).title, undefined)
assert.deepEqual(nav.at(-1).items.map((i) => i.label), ['Atlas'])
assert.deepEqual(nav[0].items.map((i) => i.label), ['Dashboard'])
assert.deepEqual(nav[3].items.map((i) => i.label), ['Account'])
})
test('a row whose `to` collides with a core row is dropped, not rendered twice', () => {
// `to` is the key the override layer stores under and React renders by. Two
// rows sharing one would give an admin a single editor row that moves both.
const warnings = []
const warn = console.warn
console.warn = (msg) => warnings.push(msg)
try {
registerNav('uo', {
area: 'public',
items: [{ label: 'Not News', to: '/site/news' }, { label: 'Atlas', to: '/uo/atlas' }],
})
const nav = withModuleNav(PUBLIC, 'public')
assert.deepEqual(nav.map((i) => i.label), ['Home', 'News', 'About', 'Atlas'])
assert.equal(warnings.length, 1)
assert.match(warnings[0], /\/site\/news.*collides/)
} finally {
console.warn = warn
}
})
test('two modules cannot claim the same path either', () => {
const warn = console.warn
console.warn = () => {}
try {
registerNav('aa', { area: 'public', items: [{ label: 'First', to: '/shared' }] })
registerNav('zz', { area: 'public', items: [{ label: 'Second', to: '/shared' }] })
const labels = withModuleNav(PUBLIC, 'public').map((i) => i.label)
assert.deepEqual(labels, ['Home', 'News', 'About', 'First'])
} finally {
console.warn = warn
}
})
test('a module row carries its moduleId through, which is how the gate finds it', () => {
registerNav('uo', { area: 'public', items: [{ label: 'Atlas', to: '/uo/atlas', feature: 'atlas' }] })
const row = withModuleNav(PUBLIC, 'public').at(-1)
assert.equal(row.moduleId, 'uo')
assert.equal(row.feature, 'atlas')
})
test('areas do not leak into one another', () => {
registerNav('uo', { area: 'admin', items: [{ label: 'Shard', to: '/admin/uo/link', group: 'System' }] })
assert.equal(withModuleNav(PUBLIC, 'public'), PUBLIC)
})
// ── The relationship that is the actual requirement ───────────────────────
test('an admin override applies to a module row exactly as to a core row', () => {
// The reason the interleave happens BEFORE the merge and not after: the merge
// drops any key its base array does not declare, so appending module rows
// afterwards would make every one of them unorderable, unrelabellable and
// unhideable — a visible regression the day the UO rows leave core.
registerNav('uo', { area: 'public', items: [{ label: 'Atlas', to: '/uo/atlas' }] })
const base = withModuleNav(PUBLIC, 'public')
const merged = applyNavOverrides(base, {
'/uo/atlas': { label: 'Bestiary', order: 0 },
'/site/news': { order: 3 },
})
assert.deepEqual(merged.map((i) => i.label), ['Bestiary', 'Home', 'About', 'News'])
})
test('an override can hide a module row, and the public tree can section it', () => {
registerNav('uo', { area: 'public', items: [{ label: 'Atlas', to: '/uo/atlas' }, { label: 'Market', to: '/uo/market' }] })
const base = withModuleNav(PUBLIC, 'public')
const hidden = buildPublicNav(base, { '/uo/atlas': { hidden: true } })
assert.equal(hidden.some((n) => n.to === '/uo/atlas'), false)
const sectioned = buildPublicNav(base, {
items: { '/uo/market': { section: 'sec_shard' } },
sections: [{ id: 'sec_shard', label: 'Shard', order: 0 }],
})
assert.equal(sectioned[0].kind, 'section')
assert.deepEqual(sectioned[0].items.map((i) => i.to), ['/uo/market'])
})
test('a module row can be moved between admin groups by an override', () => {
registerNav('uo', { area: 'admin', items: [{ label: 'Shard', to: '/admin/uo/link', group: 'System' }] })
const base = withModuleNav(ADMIN, 'admin')
const merged = applyNavOverrides(base, { '/admin/uo/link': { group: 'Moderation' } })
assert.deepEqual(merged[1].items.map((i) => i.to), ['/admin/moderation', '/admin/uo/link'])
assert.deepEqual(merged[2].items.map((i) => i.to), ['/admin/users', '/admin/settings'])
})
test('a group a module created is itself a legal override destination', () => {
// Falls out of building the destination set from the base nav it is handed —
// recorded because it is the kind of thing that would otherwise be discovered
// by an admin finding a section they cannot move anything into.
registerNav('uo', { area: 'admin', items: [{ label: 'Atlas', to: '/admin/uo/atlas', group: 'Shard' }] })
const base = withModuleNav(ADMIN, 'admin')
const merged = applyNavOverrides(base, { '/admin/users': { group: 'Shard' } })
assert.deepEqual(merged.at(-1).items.map((i) => i.to), ['/admin/uo/atlas', '/admin/users'])
})

View File

@@ -12,6 +12,7 @@ import {
routesFor,
navFor,
featureProviderFor,
featureProviders,
registeredIds,
_reset,
} from '../src/modules/registry.js'
@@ -126,6 +127,28 @@ test('a feature provider is stored under its namespace, with its owner', () => {
assert.equal(featureProviderFor('nothing'), undefined)
})
test('providers can be enumerated in registration order, with their owner', () => {
// Core's feature context has to CALL each of these, as a hook, in a fixed
// order — so it needs the list, and it needs the owner id to resolve a nav
// row whose `moduleId` says who it belongs to (modules/features.jsx).
const uo = () => null
const rust = () => null
registerFeatureProvider('uo', 'shard', uo)
registerFeatureProvider('rust', 'server', rust)
assert.deepEqual(featureProviders(), [
{ id: 'uo', namespace: 'shard', hook: uo },
{ id: 'rust', namespace: 'server', hook: rust },
])
})
test('enumerating providers is NOT part of the module-facing surface', () => {
// A module asks for a namespace it knows the name of; enumerating what
// everyone else registered is core's business, so `featureProviders` is a
// module export and not a member of window.__rg.registry.
assert.equal(registry.featureProviders, undefined)
assert.equal(typeof featureProviders, 'function')
})
test('every registration marks the module registered', () => {
registerRoutes('a', { public: [{ path: 'x' }] })
registerNav('b', { area: 'public', items: [] })