From 472e510f6f8192bb8d4c5bafe4dcd807d49af406 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Tue, 11 Aug 2026 21:55:03 -0500 Subject: [PATCH] =?UTF-8?q?docs(website):=20record=20slice=204,=20and=20th?= =?UTF-8?q?e=20=C2=A75.2=20check=20as=20built?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MODULE_SYSTEM.md §2.7.1 gains the slice 4 record: the 190 dead lines of UO API bindings slice 3 left behind (with zero consumers, which is why nothing caught them), the copy rewrite and why the words stay in code rather than becoming a config surface, the two settings rows core had no business seeding, and the check that keeps all of it out. The slice table row is corrected to what actually moved, and README.md is recorded as deliberately deferred to slice 5 rather than quietly skipped. The defect gets its own subsection with the SQL, because it is a shape that will recur: a guard in core and the statement it guards in a fragment run in the wrong order, since core's schema is replayed in full before any fragment. Verified against a real MariaDB in all three states, including the pinned-instance case that makes it a one-shot rather than a re-bump. MODULE_API.md §5.2 stops describing a grep and describes what was built — four details promoted to contract because each was wrong first: whole-word matching (a substring pass flags `defaultImage`, which contains "ultIma"), the comments-then-strings character walk, grandfathering exemptions that fail the build when they stop matching, and `git ls-files` over a directory walk. BACKEND_DESIGN.md's settings table gains the keys a MODULE seeds, and the ordering rule behind them. Co-Authored-By: Claude --- website/BACKEND_DESIGN.md | 12 ++++- website/MODULE_API.md | 23 +++++++++ website/MODULE_SYSTEM.md | 106 +++++++++++++++++++++++++++++++++++++- 3 files changed, 139 insertions(+), 2 deletions(-) diff --git a/website/BACKEND_DESIGN.md b/website/BACKEND_DESIGN.md index 2f0726d..59e4ab1 100644 --- a/website/BACKEND_DESIGN.md +++ b/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 diff --git a/website/MODULE_API.md b/website/MODULE_API.md index 061405b..0755c6b 100644 --- a/website/MODULE_API.md +++ b/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 diff --git a/website/MODULE_SYSTEM.md b/website/MODULE_SYSTEM.md index b56bfec..aa329e6 100644 --- a/website/MODULE_SYSTEM.md +++ b/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.