Merge pull request 'docs(website): record slice 4, and the §5.2 check as built' (#139) from docs/module-de-uo-core into main

Reviewed-on: #139

website/BACKEND_DESIGN.md

@@ -274,7 +274,17 @@ Seeded with the 8 spec categories: `new-player-guide, maps-atlas, systems, items
 
 Seeded keys: `site_mode` (default `maintenance`), `site_mode_changed_at`,
 `site_mode_changed_by`, `maintenance_message`, `status_message`, `homepage_teaser`,
-`contact_email` (=UOMysticmoon@gmail.com), `site_title`.
+`contact_email` (=UOMysticmoon@gmail.com), `site_title`, `player_registration`
+(default `disabled`), `mobile_app_links_enabled`.
+
+**Keys a MODULE seeds into this table.** `settings` is core's, but a module's
+schema fragment may `INSERT IGNORE` its own rows into it, and module-uo seeds two:
+`game_account_signup` (default `disabled`) and the one-shot migration marker
+`uo_link_protocol_3_migrated`. Core seeded both until Phase 3 slice 4, which is
+worth knowing for one reason beyond tidiness — a fragment runs **after** core's
+schema is replayed in full, so a marker in core guarding a statement in a fragment
+fires before the statement reads it. That exact ordering silently disabled the
+protocol-3 migration between slices 1 and 4; see MODULE_SYSTEM.md §2.7.1.
 
 **Deliberately unseeded keys** — the theming & navigation overrides
 (`theme_visual`, `brand_assets`, `nav_public`, `nav_admin`, `nav_player`). All

website/MODULE_API.md

@@ -1052,6 +1052,29 @@ declare their symbols. It can talk about them in English.
 Core's UO-flavoured default copy is dealt with directly instead, as `MODULE_SYSTEM.md` §2.7.1's
 slice 4 — a rewrite with its own review, not an exemption.
 
+**Implemented in slice 4** as `scripts/checkModuleIdentifiers.js` / `npm run check:modules`, run as
+the first step of the server-tests job. Four details are contract rather than implementation, because
+each was wrong in a first attempt:
+
+- **Matching is on whole WORDS.** Names are tokenised on camelCase humps and on `-`/`_`/`.`/`/`, and
+  compared word by word — so `shardStatus` is a hit and `defaultImage`, which contains the substring
+  "ultIma", is not. `uo`+`link` and `town`+`crier` match only as adjacent pairs; "link" on its own is
+  ordinary core vocabulary and a check that forbade it would be ignored within a week.
+- **Comments and string bodies are masked in one left-to-right character walk, comments first.** A
+  comment contains quotes and a string contains `//`; no ordering of regexps gets both right. Masked
+  rather than deleted, so offsets survive and a route path literal is still findable at its own
+  position.
+- **Grandfathering (§6.5) is exempted by an explicit list naming file, identifier and reason**, and
+  an entry that matches nothing **fails the build**. Core's `LEGACY_TABLE_PREFIXES` and
+  `LEGACY_STREAM_IDS`/`LEGACY_LEGS` are maps keyed by module id, so grandfathering cannot be written
+  down without naming who is grandfathered; that is the only sanctioned reason to add one.
+- **The file list comes from `git ls-files`**, not a directory walk: an untracked, gitignored,
+  operator-supplied data file is not core's source and must not fail anyone's build.
+
+The check has its own test suite. That is not optional here — a boundary check is written when the
+boundary is already clean, so it never fires again, and nothing distinguishes "still checking" from
+"quietly broken" without cases it is required to reject.
+
 ### 5.3 Zero-line route manifest diff (CI, both repos)
 
 `npm run routes:manifest -- --check` in core; the module generates and freezes its own manifest in

website/MODULE_SYSTEM.md

@@ -781,7 +781,7 @@ Each slice is one `module-uo` PR (adds), one `website` PR (deletes), and one `do
 | 1 | **The whole server half** | 40 files / ~9,674 lines, 25 of core's 82 test files, 27 of its 68 tables — every UO model, util, router and controller, `config/shardStreams.js`, `scripts/importSpawnAtlas.js` and the art JSON. **One merge, five commits** (below). |
 | 2 | **Client extension slots** | Core only, and the one slice that adds rather than moves: the client twin of `declareSlot`/`registerExtension` (API §3.7), the `site.footer.status` and `admin.users.detail` slots, and core filling both itself. `module-uo` untouched. |
 | 3 | **The whole client half** | **35 files / 5,332 lines** (measured; the 51/~3,700 above was counted differently) — all twelve public pages (`Shard`, `ShardActivity`, `Rules`, `Atlas`, `AtlasCreature`, `ChampSpawns`, `Market`, `MarketVendor`, `Governors`, `Guilds`, `Houses`, `Leaderboards`) under `/uo/*`, every admin and player view under `/admin/uo/*` and `/player/uo/*`, `PlayersOnline`, `VendorSales`, `CharacterStats`, `GameAccounts`, the `data/` and `lib/` UO leaves, the public nav rows, the feature provider, and all three slot fills. |
-| 4 | **De-UO core's copy** | `About`, `Screenshots`, `Website`, `SiteFooter`'s prose, `heroLayout`'s defaults, `api/client.js`'s `shard`/`atlas` namespaces, and the comments in `navOverrides.js` — plus the §5.2 CI grep that keeps them out |
+| 4 | **De-UO core's copy** | `About`, `Screenshots`, `Website`, `Status`, `Wiki`, `SiteFooter`'s prose, `heroLayout`'s defaults, `brand.js`'s tagline + description, `db/seed.js`'s wiki copy, two user-visible `NavEditor` strings, the comments in `navOverrides.js`, and the 190 dead lines of `api/client.js`'s `shard`/`atlas` namespaces — plus the two settings rows core seeded for module-uo, and the §5.2 CI check that keeps all of it out. `README.md` deliberately deferred to slice 5. |
 | 5 | **Close the phase** | `module-uo`'s frozen route manifest and release workflow; `docs/modules/uo/` and `docs/modules/rust-dryrun.md` |
 
 ##### Why the server half cannot be sliced — found 2026-08-11, before writing any of it
@@ -1125,6 +1125,110 @@ no DOM, because nothing renders. It holds the agreement that rots quietly (every
 route this module registered) rather than restating both lists. CI now builds before it tests, since
 both artifact-reading tests skip without a build and were otherwise green while asking nothing.
 
+#### Slice 4 — de-UO core's copy, and enforce it (Module-uo#5 + website#140, 2026-08-11)
+
+The slice that makes criterion 1 a fact rather than a promise. Core ends at **637 server + 157
+client** tests, a manifest unchanged at 158 public + 2 internal, and a byte-identical OpenAPI spec.
+Three separable pieces, landed together because the check is what keeps the other two true.
+
+##### The bindings were dead a whole slice before anyone noticed
+
+`client/src/api/client.js` still carried **190 lines** of UO namespaces — `shard`, `atlas`, both SSE
+URLs, `admin.shard` / `shardOps` / `atlas` / `userShard`, the uo-link and town-crier calls,
+`player.shard` — with **zero core consumers** since slice 3 deleted the views that called them.
+Nothing failed, nothing warned, and the client build was happy: an API binding with no caller is
+inert. Worth naming as a class, because the same thing is true of any leaf a deletion slice leaves
+behind, and only the §5.2 check would have found it.
+
+Five assertions went with them. Core's `apiClient.test.js` was still testing UO URLs — the
+atlas-vs-`/shard` path split, the query-string filtering, the slug encoding, the admin atlas methods
+— so deleting the bindings would have deleted the coverage too. They are `module-uo`'s
+`client/test/api.test.js` now, joined by a new one pinning the seven admin URLs the shipped Android
+app calls by name. The encoding test that used `governorHistory` was re-pointed at a core route
+rather than dropped: what it actually guards is `req`'s encoding, which is core's.
+
+##### The copy, and where a game's name belongs instead
+
+`About`, `Screenshots`, `Website`'s cards, `Wiki`, `SiteFooter`, the default hero, `brand.js`'s
+tagline and description, the seeded wiki categories, and two **user-visible** NavEditor strings that
+named a module's admin screen by its proper name ("Shard Visibility"). `Status` was the interesting
+one: it reports **site mode**, has never had anything to do with a game server, and was called "Shard
+Status" purely by habit.
+
+Nothing is lost, because core already has three places an instance says which game it is: `BRAND_*`
+(and `.env.uomysticmoon.example` sets both brand strings explicitly, so the live instance's wording
+does not change at all), the hero editor, and CMS pages. `heroLayout.defaultLayout` is both the
+rendered default *and* the editor's starting point, which is the argument for leaving the words in
+code and making them neutral rather than inventing a config surface for prose.
+
+**Wiki page slugs are deliberately untouched.** `seedDefault*` only inserts a row that is absent, so
+renaming `maps-atlas` does not rename anything — it adds a ninth page to every existing install.
+
+##### Two settings rows core had no business seeding — one of them broken
+
+`game_account_signup` and `uo_link_protocol_3_migrated` were both seeded by core's `schema.sql`. The
+first is the row slice 3 explicitly kept the *key* of; the second is a one-shot migration marker.
+Both move to module-uo's fragment, and the second was a live defect.
+
+**The marker fired before the migration that reads it.** Pre-extraction these two statements were
+adjacent in core's schema:
+
+```sql
+UPDATE uo_link_config SET protocol = 3
+  WHERE id = 1 AND protocol < 3
+    AND NOT EXISTS (SELECT 1 FROM settings WHERE `key` = 'uo_link_protocol_3_migrated');
+INSERT IGNORE INTO settings (`key`, value) VALUES ('uo_link_protocol_3_migrated', '1');
+```
+
+Slice 1 moved the `UPDATE` to module-uo and left the `INSERT` behind. The two files do not run
+together — core's schema is replayed **in full** before any module fragment (§2.6) — so the marker
+existed before the guard ever read it and the one-shot could never fire. An install carrying a
+protocol-2 row would have stayed pinned at 2 against a v3 sidecar: every REST call 409, the WS closed
+on `ws.hello`, which is precisely what the migration exists to prevent. Latent rather than live, and
+only because `edge` has not cut over: it bites an install that first boots a post-slice-1 build while
+already holding a `uo_link_config` row.
+
+Verified against a real MariaDB, all three states: a fresh database writes both rows and has nothing
+to migrate; a database with `protocol = 2` and no marker is moved to 3 and marked; and a database
+deliberately pinned back to 2 **with** the marker present stays at 2 across a restart, which is the
+half that makes it a one-shot rather than a re-bump.
+
+`schemaFragment.test.js` asserts the order, because that is the property that broke and no other kind
+of test can see it — both files were individually valid SQL and both replayed cleanly. Also gone: an
+orphan comment block in core's `schema.sql` describing the spawn-atlas tables slice 1 took away.
+
+##### The check: `scripts/checkModuleIdentifiers.js`
+
+`npm run check:modules`, and the **first** step of the server-tests job — before `npm ci`, because it
+is plain Node over `server/` and `client/` source with no dependency of its own, so a boundary break
+is the first thing a reviewer sees rather than something found under a pile of unrelated failures.
+What it reads is [API §5.2](MODULE_API.md#52-zero-uo-identifiers-in-core-ci-website-repo); what it
+learned by getting things wrong first is worth recording:
+
+- **Whole words, not substrings.** A case-insensitive substring pass flags `defaultImage`, which
+  contains "ultIma", and it did so four times in this repo on its first run. Names are tokenised on
+  camelCase humps and separators, and matched as words — with `uo`+`link` and `town`+`crier` matched
+  as adjacent *pairs*, since "link" alone is ordinary core vocabulary.
+- **Comments and strings are masked in one character walk**, comments first. This is the
+  `checkImports.js` lesson for the third time: a comment contains quotes, a string contains `//`, and
+  the two cannot be handled in either order by regexp. Masking rather than deleting, so a route path
+  literal is still findable at its own offset and reported line numbers stay honest.
+- **It has its own 17-test suite.** A boundary check that silently stops checking is worse than none,
+  and every "must not catch" case in it is a false positive an earlier version really produced.
+- **The three §6.5 grandfathering allowlists are exempt by name**, with their reason — `LEGACY_TABLE_PREFIXES`
+  and `LEGACY_STREAM_IDS`/`LEGACY_LEGS` are maps *keyed by module id*, and there is no way to write
+  grandfathering down without naming who is grandfathered. An exemption that stops matching **fails
+  the build**: the exemption list is the only part of this check that can weaken it, so it is the part
+  that gets the noise.
+- **It reads `git ls-files`, not a directory walk.** An operator's gitignored
+  `server/db/data/spawnAtlas.art.json` sits in the tree of anyone who has run the atlas import and
+  would otherwise report a file-name violation no commit could fix.
+
+**Deferred to slice 5, deliberately: `README.md`.** 48 UO mentions, including a whole
+`## Shard integration (uo-link)` section and the architecture diagram. §5.2 does not cover prose, and
+core's README is a rewrite that belongs with the phase-closing documentation pass rather than
+half-done inside a code slice.
+
 **Phase 4 — Delivery.** The admin-panel Modules screen (install, enable, disable, retry, purge,
 `startup_failed` with its recorded reason) and the Docker-environment path from §2.5. Deliberately
 last, so loader, packaging, schema and chunk-loading problems are not all being debugged at once.