test(client): check what the chunk registers, and fix two defects in the checks

Three things, all about checks that had never met a real chunk.

`checkExternals.js` rejected slice 3's build outright, naming a fragment of
minified JSX as an imported specifier: a button reading "Approve and import"
puts the token immediately before a quote, and no regexp can tell that from a
statement. Same wall the server's `checkImports.js` hit, answered the same way —
a character walk. A mask rather than a rewrite, because a real import has its
keyword outside a string and its specifier inside one.

Writing the test for that false positive found the false NEGATIVE underneath
it: the pattern required whitespace after `import`, so it could not see
`import{useState}from"react"` — the one shape a minified build actually emits,
and the most likely way for a missed alias to reach production. It has never
been able to see it.

`registration.test.js` is new: stand up a fake `window.__rg` with a recording
registry and the real React, import the BUILT chunk, and read back what it
asked for. No DOM, because nothing renders. It holds the agreement that rots
quietly — every nav row points at a route this module actually registered —
rather than restating both lists.

CI now builds before it tests, because both of those read `dist/entry.js` and
skip without it. Run the other way round they are green and asking nothing.

Co-Authored-By: Claude <noreply@anthropic.com>

client/test/build.test.js

@@ -20,6 +20,7 @@ import { fileURLToPath } from 'node:url'
 const HERE = path.dirname(fileURLToPath(import.meta.url))
 const CLIENT = path.resolve(HERE, '..')
 
+const { bareImports, problemsWith } = await import('../scripts/checkExternals.js')
 const configModule = await import('../vite.config.js')
 const config = configModule.default
 const { SHARED, SHARED_PACKAGES: guardedPackages } = configModule
@@ -92,15 +93,63 @@ test('modulePreload polyfilling stays off — an inline bootstrap is refused und
   assert.strictEqual(config.build.modulePreload.polyfill, false)
 })
 
-test('every shim reads from window.__rg and imports nothing', () => {
+test('exactly one file reads window.__rg, and every shim goes through it', () => {
+  // `shim/rg.js` is the single reader, and that is not tidiness: it is what
+  // makes the "core did not publish its dependencies" message reachable. The
+  // shims touch the global before anything else in the chunk does, so a check
+  // placed in the first-imported file is a guarantee that lasts until someone
+  // sorts the imports.
   const dir = path.join(CLIENT, 'src', 'shim')
   const shims = fs.readdirSync(dir)
-  assert.ok(shims.length >= 4)
+  assert.ok(shims.length >= 5)
   for (const file of shims) {
     const source = fs.readFileSync(path.join(dir, file), 'utf8')
-    assert.match(source, /window\.__rg/, `${file} does not read the global`)
-    // A shim that imported anything would be a shim with a dependency to
-    // resolve, which is the problem it exists to remove.
-    assert.doesNotMatch(source, /^\s*import\s/m, `${file} imports something`)
+    const code = source.replace(/^\s*\/\/.*$/gm, '') // the comments discuss the global
+    if (file === 'rg.js') {
+      assert.match(code, /window\.__rg/, 'rg.js must be the one that reads the global')
+      assert.doesNotMatch(code, /^\s*import\s/m, 'rg.js imports something')
+      continue
+    }
+    assert.doesNotMatch(code, /window\.__rg/, `${file} reads the global directly instead of via rg()`)
+    assert.match(code, /rg\(\)/, `${file} does not resolve through rg()`)
+    // A shim may import its sibling helper and nothing else — anything further
+    // would be a shim with a dependency to resolve, the problem it exists to remove.
+    for (const [, spec] of code.matchAll(/^\s*import\s[^'"]*['"]([^'"]+)['"]/gm)) {
+      assert.strictEqual(spec, './rg.js', `${file} imports ${spec}`)
+    }
   }
 })
+
+test('the built chunk has no bare imports and bundles no shared dependency', () => {
+  // The artifact check itself, over the artifact that ships. Skipped rather than
+  // failed when there is no build: `npm test` must be runnable before `npm run
+  // build`, and CI runs them in order.
+  const chunk = path.join(CLIENT, 'dist', 'entry.js')
+  if (!fs.existsSync(chunk)) return
+  assert.deepStrictEqual(problemsWith(fs.readFileSync(chunk, 'utf8')), [])
+})
+
+test('an import inside a string is not an import — the check reads code, not text', () => {
+  // The regression that made this necessary: slice 3's chunk was the first with
+  // any content in it, and a button labelled "Approve and import" put the token
+  // immediately before a quote. The check rejected the whole build, naming a
+  // fragment of minified JSX as the offending specifier.
+  const uiCopy = 'const a=n("button",{children:"Approve and import"}),b=1;'
+  assert.deepStrictEqual(bareImports(uiCopy), [])
+
+  // Neither is one in a comment, or in a template literal.
+  assert.deepStrictEqual(bareImports('// import "react" would be wrong here\nconst a=1'), [])
+  assert.deepStrictEqual(bareImports('/* import "react" */ const a=1'), [])
+  assert.deepStrictEqual(bareImports('const s=`import "react"`'), [])
+
+  // And a real one still is, in each form the build could emit.
+  assert.deepStrictEqual(bareImports('import"react";'), ['react'])
+  assert.deepStrictEqual(bareImports('import{useState}from"react";'), ['react'])
+  assert.deepStrictEqual(bareImports('const m=await import("react-dom/client")'), ['react-dom/client'])
+  // A relative specifier is a split chunk, not a shared dependency: not our concern.
+  assert.deepStrictEqual(bareImports('import"./other.js";'), [])
+
+  // The case that proves the mask tracks escapes: a quote escaped INSIDE a
+  // string must not end it early and leave the tail looking like code.
+  assert.deepStrictEqual(bareImports('const s="he said \\"import\\" loudly";'), [])
+})

client/test/regionBuckets.test.js

@@ -0,0 +1,60 @@
+import { test } from 'node:test'
+import assert from 'node:assert/strict'
+import { bucketize, BUCKETS } from '../src/data/regionBuckets.js'
+
+// Unit-test the presence.online region roll-up for the "Players Online" widget.
+// The load-bearing invariant: the bucket counts ALWAYS reconcile to the true
+// total — anything unmatched lands in Wilderness — so the widget can never show
+// a sum that disagrees with the headline online count.
+
+test('bucketize groups named regions into their buckets', () => {
+  const { rows, total } = bucketize({
+    'Britain': 4,
+    'Moonglow': 2,
+    'Despise': 3,
+    'Green Acres House 12': 1, // not a town/dungeon name → Housing
+  })
+  const byId = Object.fromEntries(rows.map((r) => [r.id, r.count]))
+  assert.equal(byId.britain, 4)
+  assert.equal(byId.towns, 2)
+  assert.equal(byId.dungeons, 3)
+  assert.equal(byId.housing, 1)
+  assert.equal(total, 10)
+})
+
+test('first match wins by BUCKETS order: a town-named house region counts as Towns, not Housing', () => {
+  // The towns regex is ^-anchored and towns is checked BEFORE housing, so a house
+  // region whose name starts with a town name is bucketed as Towns. Pinning this
+  // documents the ordering dependency for anyone retuning BUCKETS.
+  const { rows } = bucketize({ 'Trinsic House 12': 1 })
+  const byId = Object.fromEntries(rows.map((r) => [r.id, r.count]))
+  assert.equal(byId.towns, 1)
+  assert.equal(byId.housing, undefined) // empty bucket dropped
+})
+
+test('an unmatched region falls through to Wilderness so counts always reconcile', () => {
+  const { rows, total } = bucketize({ 'Some Unnamed Field': 5, 'Wilderness': 2 })
+  const wilderness = rows.find((r) => r.id === 'wilderness')
+  assert.equal(wilderness.count, 7)
+  assert.equal(total, 7)
+  // The reconciliation guarantee: the buckets sum to the total, exactly.
+  assert.equal(rows.reduce((s, r) => s + r.count, 0), total)
+})
+
+test('bucketize returns rows in BUCKETS order and drops empty buckets', () => {
+  const { rows } = bucketize({ 'Despise': 1, 'Britain': 1 })
+  assert.deepEqual(rows.map((r) => r.id), ['britain', 'dungeons']) // BUCKETS order, no empty towns/housing/wilderness
+})
+
+test('bucketize coerces non-numeric counts and tolerates empty/nullish input', () => {
+  assert.deepEqual(bucketize({}), { rows: [], total: 0 })
+  assert.deepEqual(bucketize(), { rows: [], total: 0 })
+  const { total } = bucketize({ 'Britain': '3', 'Minoc': 'oops' })
+  assert.equal(total, 3) // '3' → 3, 'oops' → 0
+})
+
+test('the last bucket is the catch-all (its match accepts anything)', () => {
+  const last = BUCKETS[BUCKETS.length - 1]
+  assert.equal(last.id, 'wilderness')
+  assert.equal(last.match('literally anything'), true)
+})

client/test/registration.test.js

@@ -0,0 +1,188 @@
+// ── What the chunk registers, checked without a browser ────────────────────
+//
+// `build.test.js` says the honest thing about this half: its real failures are
+// timing and resolution, and a DOM-less runner cannot see either. That is still
+// true, and MODULE_API.md §7.7's browser smoke is still what proves the module
+// works. But it left a gap worth closing, and slice 3 is when it started to
+// matter: nothing checked *what* the chunk registers.
+//
+// It can be checked, because registration is the one thing this chunk does at
+// evaluation time and it does it through an object core hands it. So: stand up a
+// fake `window.__rg` with a recording registry and the real React behind it,
+// import the BUILT artifact, and read back what it asked for. No DOM is needed
+// because nothing renders — `<Shard />` is `jsx(Shard)`, an object, and the
+// route table is full of them by design.
+//
+// What this catches that review does not: a page that silently stops being
+// routed, a nav row whose `to` drifts from its route's path, a slot fill that
+// was renamed on one side, and the whole registration surface disappearing
+// because an exception was thrown halfway down entry.jsx.
+//
+// What it deliberately does NOT do is re-assert the paths as a literal list.
+// The interesting property is that the nav and the routes AGREE, and a test that
+// restates both is a second copy of the thing it is checking.
+
+import test from 'node:test'
+import assert from 'node:assert/strict'
+import fs from 'node:fs'
+import path from 'node:path'
+import { fileURLToPath } from 'node:url'
+
+import * as react from 'react'
+import * as jsxRuntime from 'react/jsx-runtime'
+import * as router from 'react-router-dom'
+
+const HERE = path.dirname(fileURLToPath(import.meta.url))
+const CHUNK = path.resolve(HERE, '..', 'dist', 'entry.js')
+
+// A component, as far as the registry cares. The kit's real members are core's;
+// nothing here renders, so a named stub is enough to be imported and passed on.
+const stub = (name) => Object.assign(() => null, { displayName: name })
+
+function fakeRg() {
+  const routes = { public: [], admin: [], player: [] }
+  const nav = { public: [], admin: [], player: [] }
+  const providers = new Map()
+  const extensions = new Map()
+  return {
+    version: '1.3.0',
+    react,
+    jsxRuntime,
+    router,
+    // `react-dom/client` is imported for the identity check in core.js and never
+    // called — createRoot in a DOM-less process would throw. The shim reads this
+    // object, so the check compares against whatever is here.
+    reactDom: { createRoot: () => { throw new Error('not in a browser') } },
+    ui: Object.fromEntries(
+      ['PublicLayout', 'PageHeader', 'Loading', 'ErrorState', 'EmptyState', 'useAsync', 'useAuth', 'useSite']
+        .map((n) => [n, stub(n)]),
+    ),
+    api: { request: async () => ({}), ApiError: Error, BASE: '/api/v1' },
+    registry: {
+      registerRoutes(id, byArea) {
+        for (const [area, list] of Object.entries(byArea || {})) {
+          for (const r of list || []) routes[area].push({ ...r, path: `${id}/${r.path}`, moduleId: id })
+        }
+      },
+      registerNav(id, { area, items }) {
+        for (const item of items || []) nav[area].push({ ...item, moduleId: id })
+      },
+      registerFeatureProvider(id, namespace, hook) { providers.set(namespace, { id, hook }) },
+      registerExtension(id, slot, Component) {
+        if (extensions.has(slot)) throw new Error(`slot "${slot}" already filled`)
+        extensions.set(slot, { id, Component })
+      },
+      routesFor: (area) => routes[area],
+      navFor: (area) => nav[area],
+    },
+    _read: () => ({ routes, nav, providers, extensions }),
+  }
+}
+
+// Loaded once: an ES module is evaluated a single time per process however many
+// times it is imported, so every test below reads the same registration pass —
+// which is also how it behaves in a browser.
+let registered = null
+let skip = false
+
+if (!fs.existsSync(CHUNK)) {
+  skip = true
+} else {
+  const rg = fakeRg()
+  globalThis.window = { __rg: rg }
+  await import(`${new URL(`file://${CHUNK.split(path.sep).join('/')}`)}`)
+  registered = rg._read()
+}
+
+const it = (name, fn) => test(name, { skip: skip && 'no dist/entry.js — run npm run build' }, fn)
+
+it('registers routes in all three areas, namespaced under the module id', () => {
+  const { routes } = registered
+  assert.equal(routes.public.length, 12)
+  assert.equal(routes.admin.length, 7)
+  assert.equal(routes.player.length, 2)
+  for (const area of ['public', 'admin', 'player']) {
+    for (const r of routes[area]) {
+      assert.match(r.path, /^uo\//, `${area} route "${r.path}" is not under the module namespace`)
+      assert.ok(r.element, `${area} route "${r.path}" has no element`)
+    }
+  }
+})
+
+it('every route path is distinct within its area', () => {
+  // Two routes on one path is a page that can never be reached, and React
+  // renders the first without complaint.
+  for (const [area, list] of Object.entries(registered.routes)) {
+    const paths = list.map((r) => r.path)
+    assert.equal(new Set(paths).size, paths.length, `duplicate path in ${area}`)
+  }
+})
+
+it('every nav row points at a route this module actually registered', () => {
+  // The agreement that matters, and the one that rots quietly: a row survives a
+  // route rename and becomes a link to core's catch-all redirect. Nav rows carry
+  // the FULL rendered path (`/uo/shard`), routes carry the namespaced one
+  // (`uo/shard`), and reconciling them is the whole test.
+  const rendered = {
+    public: (p) => `/${p}`,
+    admin: (p) => `/admin/${p}`,
+    player: (p) => `/player/${p}`,
+  }
+  for (const [area, rows] of Object.entries(registered.nav)) {
+    const reachable = new Set(registered.routes[area].map((r) => rendered[area](r.path)))
+    for (const row of rows) {
+      assert.ok(
+        reachable.has(row.to),
+        `${area} nav row "${row.label}" links to ${row.to}, which no route serves`,
+      )
+    }
+  }
+})
+
+it('admin nav rows carry an icon; core rows all have one and a text-only row reads as breakage', () => {
+  for (const row of registered.nav.admin) {
+    assert.equal(typeof row.icon, 'function', `admin nav row "${row.label}" has no icon`)
+  }
+})
+
+it('a nav row that gates on a feature is gated by a namespace this module provides', () => {
+  // Resolution is by the REGISTERING module (§3.3), so a `feature` on a row from
+  // a module that registered no provider resolves against nothing — and
+  // everything fails open, which would re-advertise surfaces an operator hid.
+  const gated = Object.values(registered.nav).flat().filter((r) => r.feature)
+  assert.ok(gated.length > 0)
+  assert.ok(registered.providers.has('uo'), 'rows carry feature gates but no provider was registered')
+})
+
+it('fills the three extension slots, each with a component', () => {
+  const { extensions } = registered
+  assert.deepEqual(
+    [...extensions.keys()].sort(),
+    ['admin.users.detail', 'player.invite.accepted', 'site.footer.status'],
+  )
+  for (const [slot, { id, Component }] of extensions) {
+    assert.equal(id, 'uo', `${slot} was filled under the wrong owner id`)
+    assert.equal(typeof Component, 'function', `${slot} was not filled with a component`)
+  }
+})
+
+it('the manifest\'s declared server slot is one this module fills', () => {
+  // module.json declares SERVER slots and the loader validates them before the
+  // chunk is ever served. Client slots cannot be declared there — the server has
+  // no knowledge of them — so this is the one place the two halves are compared.
+  const manifest = JSON.parse(fs.readFileSync(path.resolve(HERE, '..', '..', 'module.json'), 'utf8'))
+  for (const slot of manifest.extensions || []) {
+    assert.ok(registered.extensions.has(slot), `module.json declares "${slot}" and the chunk does not fill it`)
+  }
+})
+
+it('registers under exactly one module id, matching the manifest', () => {
+  const manifest = JSON.parse(fs.readFileSync(path.resolve(HERE, '..', '..', 'module.json'), 'utf8'))
+  const owners = new Set([
+    ...Object.values(registered.routes).flat().map((r) => r.moduleId),
+    ...Object.values(registered.nav).flat().map((r) => r.moduleId),
+    ...[...registered.extensions.values()].map((e) => e.id),
+    ...[...registered.providers.values()].map((p) => p.id),
+  ])
+  assert.deepEqual([...owners], [manifest.id])
+})

client/test/shardEvents.test.js

@@ -0,0 +1,74 @@
+import { test } from 'node:test'
+import assert from 'node:assert/strict'
+import { describe, categoryOf, kindLabel, CATEGORIES } from '../src/lib/shardEvents.js'
+
+// Unit-test the shared shard-event formatter — the single place that decides how
+// each event kind reads and which filter category it belongs to. These strings
+// are user-facing on the public Shard page, the Activity feed, and the admin
+// live feed, so a regression here is visible everywhere at once.
+
+// ── describe(): works on both stored (.payload) and live (top-level) frames ──
+test('describe reads fields from .payload when present, else the top level', () => {
+  const stored = { kind: 'quest.complete', payload: { who: { name: 'Ada' }, quest: 'The Cavern' } }
+  const live = { kind: 'quest.complete', who: { name: 'Ada' }, quest: 'The Cavern' }
+  assert.equal(describe(stored), 'Ada completed “The Cavern”')
+  assert.equal(describe(live), 'Ada completed “The Cavern”')
+})
+
+test('describe resolves an actor from name → acct → "Someone"', () => {
+  assert.equal(describe({ kind: 'mob.login', who: { name: 'Bob' } }), 'Bob entered the world')
+  assert.equal(describe({ kind: 'mob.login', who: { acct: 'acct7' } }), 'acct7 entered the world')
+  assert.equal(describe({ kind: 'mob.login', who: null }), 'Someone entered the world')
+  assert.equal(describe({ kind: 'mob.login', who: 'RawString' }), 'RawString entered the world')
+})
+
+test('describe pluralizes a vendor sale only when amount > 1 and formats the price', () => {
+  assert.equal(describe({ kind: 'vendor.sale', itemType: 'Katana', amount: 1, price: 1200 }), 'Katana sold for 1,200gp')
+  assert.equal(describe({ kind: 'vendor.sale', itemType: 'Arrow', amount: 40, price: 80 }), 'Arrow ×40 sold for 80gp')
+})
+
+test('describe includes the killer only when present (optional clause)', () => {
+  assert.equal(describe({ kind: 'player.death', who: { name: 'Ada' } }), 'Ada was slain')
+  assert.equal(
+    describe({ kind: 'player.death', who: { name: 'Ada' }, killer: { name: 'Orc' } }),
+    'Ada was slain by Orc',
+  )
+})
+
+test('describe champ.update branches on status and boss state', () => {
+  assert.equal(describe({ kind: 'champ.update', name: 'Rikktor', status: 'active', bossUp: true }), 'Rikktor: boss is up')
+  assert.equal(
+    describe({ kind: 'champ.update', name: 'Rikktor', status: 'active', level: 3 }),
+    'Rikktor is active — level 3',
+  )
+  assert.equal(describe({ kind: 'champ.update', name: 'Rikktor', status: 'cooldown' }), 'Rikktor is on cooldown')
+})
+
+test('describe falls back to the raw kind for an unknown event', () => {
+  assert.equal(describe({ kind: 'some.future.kind' }), 'some.future.kind')
+})
+
+// ── categoryOf(): membership + catch-all ────────────────────────────────
+test('categoryOf groups kinds per the CATEGORIES table, and unknowns are "other"', () => {
+  assert.equal(categoryOf('player.death'), 'pvp')
+  assert.equal(categoryOf('skill.gain'), 'progress')
+  assert.equal(categoryOf('house.decay'), 'world')
+  assert.equal(categoryOf('vendor.sale'), 'other') // deliberately not a public category
+  assert.equal(categoryOf('totally.unknown'), 'other')
+})
+
+test('every kind listed in CATEGORIES maps back to that category (table stays consistent)', () => {
+  for (const cat of CATEGORIES) {
+    if (!cat.kinds) continue
+    for (const kind of cat.kinds) {
+      assert.equal(categoryOf(kind), cat.id, `${kind} should be in ${cat.id}`)
+    }
+  }
+})
+
+// ── kindLabel(): badge text ─────────────────────────────────────────────
+test('kindLabel turns dots/underscores into spaces and tolerates empty input', () => {
+  assert.equal(kindLabel('player.death'), 'player death')
+  assert.equal(kindLabel('account.login.attempt'), 'account login attempt')
+  assert.equal(kindLabel(null), '')
+})