A single recordFailure() locks for BASE_MS * 2 ** 0 — exactly one second — and
the test then does a real HTTP round trip against it. On CI that round trip took
1,456 ms and the guard correctly answered 200, failing the run for a reason that
has nothing to do with what the test is about.
Five failures lock for sixteen seconds. The subject is the guard's answer while
locked out, which is unchanged.
Co-Authored-By: Claude <noreply@anthropic.com>
MODULES declares the module set a deployment runs, one entry per module as
`<id>@<version>=<install manifest URL>`, and the container arrives at it by
itself (MODULE_SYSTEM.md §2.7.2 decision 4). A module already unpacked at the
declared version is a no-op that makes NO network call, so a restart with the
network down comes up unchanged; anything else goes through install.js — same
allowlist, same sha256, same inspect-then-extract — and install() now takes an
`expect: {id, version}` so a URL resolving to another module or version is
refused while it is still only a manifest.
Resolution runs inside start(), between the seed and the require of app.js: the
seed is where the host allowlist setting comes from, and the require is what
scans the volume. That buys it the database, so a compose-installed module gets
the same provenance columns an admin install writes.
A failure is logged and carried, never fatal — an unreachable release host must
not take the site down. The declaration owns what is on the volume; the row owns
whether a module runs, so uninstalling a declared module returns its files at
the next start and leaves it disabled. The admin list gains that as a fourth
source (declared / declaredVersion / declaredError), because a declared module
that failed to resolve has no row, no directory and nothing mounted.
Deferring the app require moved core's schema ahead of the volume scan, and the
module schema-fragment replay was wired to core's schema — so every installed
module silently got no tables. Invisible to the suite (each one stubs the loader
or the pool) and to a smoke on a database that already had the tables; found by
booting against an empty one. ensureSchema() now takes `replayModules: false`
for the one caller that scans later, server.js replays them itself after the
require, and a bootOrder test pins the five steps in the only order they work in.
741 server tests (+18), 187 client (+5); manifest unchanged at 166 public + 2
internal, OpenAPI byte-identical.
Co-Authored-By: Claude <noreply@anthropic.com>
The screen slice 1's API was written for: install from a release URL, enable,
disable, uninstall, purge, and restart. Admin-only, matching the server, and
core's own screen because it is how a module reaches the volume at all.
182 client tests (+21), manifest and OpenAPI unchanged.
Everything that decides what a row SAYS and which buttons it offers is in
`lib/moduleAdmin.js` -- plain JS, so the DOM-less runner can reach it, the
same reason `lib/adminNav.js` is. The JSX renders what it returns.
Three sources of truth, and they are allowed to disagree
--------------------------------------------------------
The row records what the operator decided and what the last boot did; the
loader says what is mounted and answering; the volume says whether there is a
directory at all. Picking one and rendering it is simpler and lies. The case
that makes it concrete is the one decision 3 creates on purpose: disable a
module (its onShutdown runs) and enable it again, and the row says `enabled`
while the loader still says `disabled` because nothing can start it before a
restart. Neither "Running" nor "Disabled" is true; "Restart to start" is.
Two shapes that are deliberately unlike the rest of the panel: the restart is
a BANNER, because a restart is a property of the server rather than of a
module and an operator who installed three modules should restart once; and
purge is offered inside the uninstall flow as a second confirm, because
purge.sql lives inside the directory being deleted and there is no later.
What the browser found that no test could
-----------------------------------------
Installing over a row the previous boot had left `startup_failed` rendered
"Failed at the require stage: module directory not present on the volume" one
second after the files had been written to the volume -- and, because that
branch is not pending, it suppressed the restart banner the install had just
told the operator to use. Every unit test passed, because none of them had
modelled a stale row plus a fresh install.
The fix is a derivation rather than a special case: the loader scans the
volume once at require time, so a module that is on the volume now and has no
live record arrived after that scan, and everything the row says about it
predates the install. That check runs before the failure one.
The same class, one place further on: an upgrade leaves the old code loaded,
so the row's version is a promise about the next boot. `liveVersion` (slice 1)
lets the screen say "Restart to finish upgrading" instead of reporting the new
version as running.
Verified against a live server and the real published release: pasted the
v0.3.0 install-manifest URL, restarted, watched the module register its five
mounts and seven streams and its own nav rows appear in the sidebar. Disable
ran its onShutdown for real -- the uo-link WebSocket closed, its routes went
to 404, and it left /public/modules -- and enable then showed the decision-3
state with the banner. The restart button itself was exercised through its
endpoint rather than clicked, because a window.confirm wedges the browser
automation.
Co-Authored-By: Claude <noreply@anthropic.com>
Standing the slice-2 screen up against a live server and installing the
published module-uo v0.3.0 through it found three things, none of which any
unit test in this repo could have caught. Two of them are older than this
phase.
1. The boot refresh nulled every install's provenance
--------------------------------------------------------
`installed_modules.source` and `.sha256` exist so the admin panel can say
where a module came from. They never survived a restart.
`lifecycle.boot()` re-records every scanned module with no source and no
sha256 -- correctly, because a scan finds a directory and never where it came
from -- and `upsert` assigned both columns unconditionally. So an install's
provenance lasted exactly until the restart that install asked for, and the
screen then described a module installed from a URL as "placed on the volume
by hand". Verified live: install, restart, provenance gone.
Nothing could have caught it before now. Phase 4 wrote the first non-null
value these columns had ever had, so lifecycle.js's comment asserting that
"recordInstalled leaves what it is not given" described an intention rather
than the statement below it -- and modules.model.test.js's fake reproduced
the defect faithfully, assigning unconditionally just like the SQL.
Fixed with COALESCE(VALUES(col), col): a value overwrites, a NULL leaves what
is there. The fake now matches, and two tests pin both directions -- a boot
refresh must not wipe it, and a re-install from a new URL must still replace
it, or the column would become write-once and an upgrade would for ever show
where the first version came from.
2. The restart killed the server on Windows instead of stopping it
------------------------------------------------------------------
The route called `process.kill(process.pid, 'SIGTERM')` to reach server.js's
graceful-shutdown handler. That works on Linux. **Windows has no POSIX
signals, and Node documents SIGTERM there as unconditional termination of the
target process** -- so on a Windows host the restart killed the server
outright: no module onShutdown, no listener close, no pool close, no log
flush. Observed exactly that: the process was gone and the shutdown handler
had logged nothing at all.
`process.on('SIGTERM', ...)` is an ordinary EventEmitter listener, so
`process.emit('SIGTERM')` reaches the same handler on every platform without
involving the OS. One shutdown path, still; it just gets there by an event.
Deployment is Linux containers and would never have shown this. Development
is not, and neither is the smoke that found it.
The test was worse than useless: it stubbed `process.kill` and asserted it
had been called with SIGTERM, which is precisely the call whose MEANING
differs by platform. It now waits for the SIGTERM EVENT -- what server.js is
actually subscribed to -- so a pass here means the handler would run.
3. `present()` did not publish the running version
--------------------------------------------------
An upgrade writes new files and a new row while the old code stays loaded, so
the row's version is a promise about the next boot rather than a description
of this one. Adds `liveVersion` from the loader beside `liveState`, so the
screen can tell the two apart instead of reporting the new version as running.
723 server tests (+2), manifest and OpenAPI both unchanged.
Co-Authored-By: Claude <noreply@anthropic.com>
The consumer half of a release module-uo's CI has been publishing since
phase 3 closed. Before this, core had the installed_modules provenance
columns and no code that could ever fill them: nothing fetched, verified,
unpacked, removed or purged anything, and there was no admin route at all.
Adds modules/archive.js, modules/install.js, schema.runPurge(),
lifecycle.stop(), loader.stopHook(), and /api/v1/admin/modules with eight
routes. 797 server tests (+76), manifest 158 -> 166 + 2 internal, OpenAPI
gains 8 operations and loses nothing.
Reject, never sanitise
----------------------
The download is the easy part: an https-only allowlist re-checked on every
redirect hop, a declared sha256 compared against the bytes that arrived, and
a byte cap. Unpacking is where the archive chooses the filenames, and core
writes into a directory bind-mounted from the host, so an escape is not
confined to the container.
archive.js inspects the whole archive before a byte is unpacked and refuses
absolute and drive-absolute paths, `..` segments, NUL bytes, backslashes,
anything that is not a regular file or a directory, more than one top-level
entry, and anything over the entry or byte caps. Refusing symlinks and
hardlinks outright is what keeps this off the majority of node-tar's
published advisories rather than depending on the library to contain them.
That two-pass shape is load-bearing, and it was measured rather than assumed:
extracting an archive whose fourth member escapes upward throws under
node-tar 7.5.22 -- and leaves the first three members on disk. The loader
scans that directory at require time on the next boot, so a half-unpacked
module is a module. Everything therefore happens in a scratch directory that
is removed on any failure, and the move into place is the last step.
`tar` is pinned to ^7.5.22 rather than the ^6 that installs by default: 6.x
is flagged critical, and reading the advisory list is what the file's header
now says out loud -- almost all of it is hardlink or symlink traversal and
PAX header interpretation differentials, which is exactly this feature's
threat model.
Two things the plan had wrong
-----------------------------
The bundle's top-level directory is `module-uo-<version>`, not the module id
-- so "the top-level name must equal the id" was checked against nothing real.
The extractor strips that level instead, because its name belongs to whoever
published the bundle and the directory it lands in is core's. What is checked
instead is the unpacked module.json: a manifest promising `uo` and delivering
something else is refused rather than installed under the name it promised.
And purge cannot be a follow-up action (decision 5): purge.sql lives inside
the directory uninstall deletes. It is offered in the uninstall flow and as a
standalone action on a still-installed module, and the standalone one refuses
unless the module is already disabled -- dropping tables under something that
is still serving leaves it answering out of a world that no longer exists.
Disable now means stopped
-------------------------
lifecycle.stop() dispatches that one module's onShutdown before flipping the
guard, so a module an operator switches off actually releases its sockets and
closes its streams instead of merely becoming unreachable. The hook runs
first and the state moves after it, because while onShutdown runs the module
is still `started` and that is the only state in which its routes and the
world it is tearing down agree. A hook that throws does not stop the disable
-- the opposite of the boot path's rule, and deliberately.
Enable is not its mirror and there is no start(id) beside it. There is no
onBoot re-dispatch and the hooks were never promised re-entrant, so enable
moves the row and the restart route starts it. A test pins that enable does
not touch the loader, because "fixing" it is a one-line change that would put
a module with closed sockets back on the nav.
Restart raises SIGTERM against its own process rather than calling the
shutdown path directly, so server.js's handler stays the one graceful-shutdown
path and this route cannot drift from it.
The allowlist bootstraps from MODULE_SOURCE_HOSTS into a settings row and is
admin-managed after that (decision 6); seedDefault is INSERT IGNORE, so
changing the variable on an existing deployment is a no-op by design. An empty
list forbids every install rather than allowing every host -- the safe
direction for a value someone might blank by accident.
Verified against the real v0.3.0 release
----------------------------------------
Not a fixture: fetched the published install manifest over the real Gitea
host and its redirect chain, verified the sha256, inspected and unpacked the
252,517-byte artifact to 82 files, and then booted core against the result --
the module registered its five mounts, seven streams and eight capabilities
and resolved its client chunk, with no scratch directory left behind.
Two defects this slice's own tooling caught, both of which had already been
written down as classes:
- the controller destructured runPurge at require time, capturing the
function rather than the module, which made the one dependency whose
ORDER matters the one that could not be substituted;
- two swagger annotations carried an apostrophe inside a quoted string,
dropped silently by swagger-autogen before slice 5 taught it to fail loudly.
Co-Authored-By: Claude <noreply@anthropic.com>
Core's half of the slice that closes phase 3. Two things: the request-time
fragment merge core has owed since phase 1, and the last of core's UO copy.
**The merge (MODULE_API.md §6.1a).** `swagger-output.json` is core's own routes
and cannot be anything else — it is generated on a developer's machine and
committed, so it must come out the same regardless of what they had checked out,
and a module arrives on a volume long after the image was built. Module routes
therefore reach the document at request time, from the `swagger-fragment.json`
each module ships: `swagger/docsSpec.js` merges the fragments of STARTED modules
over the committed spec, cached on a new loader state version and rebuilt when a
module's state moves.
Until now neither half existed. `swagger/mergeSpec.js` named the request-time
caller in its header and that caller was never written, so the 72 routes
module-uo serves were in no OpenAPI spec at all — core's standing rule ("never
ship a route that isn't in the spec") broken by the extraction rather than by a
route.
Core wins every key collision, `swagger-output.json` is never mutated (it is a
require()d JSON module — one in-place merge would be permanent AND cumulative),
and a fragment that is missing or unreadable costs that module its paths and
nothing else. The Swagger UI is now built per request for the same reason the
JSON is: bound once at require time it would show core's routes for the life of
the process while /api/docs.json showed the merged set.
**The last of core's UO copy** (slice 4 deferred it; §5.2's check reads code, not
prose, so none of this was caught):
- 31 UO schemas and 4 UO tags in `swagger/swagger.js`, describing routes core has
not served since slice 1 — 578 lines. They moved to module-uo, namespaced
`Uo…`, and arrive back through the merge on an instance that installs it.
- `info.description` said "a private Ultima Online shard".
- README.md's 48 UO mentions, including the architecture diagram and the whole
`## Shard integration (uo-link)` section, now `## Modules`.
- `TOWNCRIER_DURATION_SEC` and `UOLINK_*` in the two `.env.example`s: read by the
module, not by core, and documented in the module's README instead.
**Two dropped annotations, and the reason nobody knew.** swagger-autogen reports
an annotation it cannot parse and then prints Success in green, having skipped
it. `npm run swagger` now captures its diagnostics and fails — which immediately
found `POST /api/v1/admin/invites` and `POST /api/v1/auth/invite/:token/accept`
documented with an EMPTY request body, both since the day they were written.
Fixing the tag list also cleared five tags used by routes but never declared
(`Admin · Email`, `Admin · Invites`, `Admin · Moderation`, `Admin · Pages`,
`Auth · Me`) — the same defect class, in the other direction.
- 646 server tests (+9), 157 client tests unchanged
- routes.manifest.json unchanged (158 public + 2 internal); check:modules clean
- swagger-output.json: 128 paths, 69 schemas, 0 orphan tags, 0 orphan schemas
- verified against a real boot with module-uo installed: 197 merged paths
(128 core + 69 module), all four module tags, 31 Uo schemas, no dangling $refs,
/api/docs renders the module's operations with zero console errors
Refs: docs/website/MODULE_API.md §2.8, §6.1a; MODULE_SYSTEM.md §2.7.1
Co-Authored-By: Claude <noreply@anthropic.com>
Phase 3's acceptance criterion 1, made real. Three things, one review:
**The dead bindings.** `client/src/api/client.js` still carried ~190 lines of UO
namespaces — `shard`, `atlas`, the two 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. module-uo vendors its own bindings.
The five assertions core's `apiClient.test.js` made about those URLs moved with
them (Module-uo#5); the encoding test that used `governorHistory` now uses a
core route.
**The copy.** Core is the platform, not one game's site, so its words are
game-neutral now: `About`, `Screenshots`, `Website`'s cards, `Status` (which was
never about a game server at all — it reports site mode), `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. Which game an instance is for is the operator's to
say — BRAND_* vars, the hero editor, CMS pages — and every real instance already
does: `.env.uomysticmoon.example` sets both brand strings explicitly, so nothing
live changes wording. Wiki page SLUGS are untouched: `seedDefault*` only inserts
what is absent, so renaming one adds a duplicate page to every install.
Also gone: an orphan comment block in `schema.sql` describing the spawn-atlas
tables slice 1 took away, and the two settings rows core seeded for a module
(`game_account_signup`, `uo_link_protocol_3_migrated`). The second was a live
defect — see Module-uo#5, which takes ownership of both and repairs the
one-shot migration core's ordering had disabled.
**The check.** `scripts/checkModuleIdentifiers.js` + `npm run check:modules`,
first step of the server-tests job because it needs no dependencies. It reads
CODE, not prose — file names, import specifiers, route path literals, declared
identifiers and property names — per §5.2, so core's English may still say
"shard" where saying it is worth more than the word costs.
Two things it gets right only because getting them wrong was tried first: it
matches WHOLE WORDS (a substring pass flags `defaultImage`, which contains
"ultIma", four times in this repo), and it strips comments and string bodies in
one character walk (a comment contains quotes, a string contains `//`) — the
`checkImports.js` lesson. It has its own 17-test suite, because a boundary check
that silently stops checking is worse than none. The three §6.5 grandfathering
allowlists are exempt by name, and an exemption that stops matching fails the
build rather than lingering.
BREAKING CHANGE: core no longer seeds `game_account_signup` or
`uo_link_protocol_3_migrated`; module-uo's schema fragment does. An install
running core without module-uo keeps whatever rows it already has and gains no
new ones — nothing in core reads either key.
Deferred to slice 5, deliberately: README.md's 48 UO mentions, including a
`## Shard integration (uo-link)` section and the architecture diagram. That is
documentation, which §5.2 does not cover, and it belongs with the phase-closing
docs pass rather than half-done here.
Co-Authored-By: Claude <noreply@anthropic.com>
Both found by the §7.7 browser smoke, running the slice-3 pair together, and
neither is visible to any test in either repo.
`PlayerPortalLayout` rendered `<n.icon />` unguarded while `AdminLayout` guarded
its equivalent. `icon` is optional in the nav contract, and every core row in
that sidebar has always had one — so the difference cost nothing until a module
registered a row without, and then it was not a missing glyph, it was React
error #130 and a blank player portal. Guarded now, like its neighbour.
`MODULES_DIR` is resolved absolute. `resolveClient` checks containment by
comparing an absolute `path.resolve(dir, entry)` against the module directory,
so a RELATIVE `MODULES_DIR` — which is what §7.7's own recipe produces when run
from `server/` — failed every module with "client.entry escapes the module
directory". A perfectly-placed entry, and a message pointing at the module.
Co-Authored-By: Claude <noreply@anthropic.com>
35 files and 5,332 lines out — twelve public pages, seven admin views, two
player views, eight components, the two `data/` leaves and the three `lib/`
ones, plus the two tests that came with them. §2.7.1's estimate of 51 files /
~3,700 lines was measured differently and is corrected in the docs PR.
The seams core keeps, each smaller than what it replaced:
Nine rows leave the public header and six leave the admin sidebar, and both
lists are now free of `feature` gates and of `IconShard`. `moduleTitle` already
handled a module page's heading, so the six TITLES entries and the
`/admin/characters` branch of `sectionTitle` simply go.
`/player` had `PlayerCharacters` as its index — a UO page — and rather than name
a replacement or invent a landing screen it now resolves to the first row of the
portal nav this viewer can reach (`firstDestinationFor`, beside
`allowedPathsFor` and reading the BASE nav for the same reason: an override is
presentation and where everybody lands is behaviour). With the module installed
that is still Characters, so a player's first screen after signing in does not
change. Deliberately generic and deliberately not in the portal layout — the
admin index is the same question with a hardcoded answer, and if the two
logged-in areas ever become one this is what serves both.
`game_account_signup` goes with the rest of core's UO prose: the mode list, the
derived public flag, the validation and a Site Settings field whose help text
named Bridge.cfg. The row itself is untouched and module-uo reads it through
ctx.settings — the data stays, the semantics move.
KNOWN BREAK, accepted by the org lead: the shipped Android app reads
`gameAccountSignup` off `/public/settings` (PublicDto.kt:80). The field has a
`= false` default so nothing crashes; the app silently stops offering
game-account creation until it reads the module's `/public/shard/features`
instead. Out of scope here, recorded in the Android plan, and it lands well
before this workstream's cutover reaches `main`.
620 server + 161 client tests. Manifest 158 public + 2 internal, unchanged;
routes.guards unchanged. The OpenAPI spec loses exactly one property, and only
because it was hand-written in swagger.js — regeneration alone would have left
the spec documenting a field core no longer returns.
Co-Authored-By: Claude <noreply@anthropic.com>
The three things core owes the client half before it can leave, all additive,
all MODULE_API 1.2.0 → 1.3.0.
`player.invite.accepted` is the third extension slot. Core's invite page owned a
UO game-account step — it read a `gameAccountSignup` flag out of core's own
settings and posted to a shard route — and an invite is a core concept that
staff receive too, so the page stays and its optional next step becomes a slot.
Named for the place, like the other two. Whether there is a step at all is the
filling module's call, made from data core does not have; core keeps the shell,
the skip control and the destination.
`icon` on a nav item, because without it the six extracted UO rows would have
been the only text-only entries in a sidebar where every other row has a glyph.
Core supplies no fallback — an invented one is core making a presentation choice
for content it knows nothing about. `icon` was already among the fields an
override may not touch, so the concept predates a module being able to send one.
`api.BASE` was in §3.5 from the first draft and never actually published.
`request` is fetch-only, so an EventSource builds its own URL, and the shard's
live feed is two of them; the alternative is a module hardcoding `/api/v1`,
which asserts something about core that core has not promised.
`AcceptInvite` is the one legitimate reader of `extensionFor` outside Slot.jsx:
the answer decides a NAVIGATION, not a decoration. Decoration goes inside
`<Slot wrap>`, which is why `hasExtension` stayed deleted.
Co-Authored-By: Claude <noreply@anthropic.com>
Core declares site.footer.status and admin.users.detail in main.jsx and fills
both itself, under owner id `core` -- the client twin of registries.registerCore()
and the same trick useShardFlags already uses. The rendered page is unchanged;
what changes is that the content now arrives the way a module's will.
The footer's Shard Status link becomes ShardStatusLink.jsx, and UserDetail's six
UO sections become UserShardSections.jsx. Both are files rather than inline
markup so that the client half of phase 3 deletes a registration and a file
instead of editing a core page under extraction pressure -- which is also what
proves the mechanism before anything depends on it.
The user-detail slot is handed userId and not scope. api.admin.userShard is a UO
binding that leaves core with the client half, so a slot passing it would hand a
module something core is about to delete; an extension builds its own client for
the routes it registered at the other end. Core's own fill now does exactly what
the module will.
Verified in a browser against a real chunk (MODULE_API.md 7.7): a throwaway
module fills both slots and renders its own label and target in the footer with
core's linkStyle, and receives userId on the admin page; a deliberate render
failure is contained to that one spot with the slot named in the console; core's
own fills leave the pages byte-identical to before; and with no module installed
both slots render nothing. Zero CSP reports throughout.
Co-Authored-By: Claude <noreply@anthropic.com>
The client twin of the server's declareSlot/registerExtension, and the same rule
in both halves: core declares a slot, only core declares one, and at most one
module fills it. Core renders <Slot name> and gets nothing back when the slot is
unfilled, so an instance with no module installed renders exactly what it
rendered before -- the same untouched-path guarantee withModuleNav makes.
A slot is named for a PLACE, never for a meaning. Core supplies the position and
the styling; the label, the target, the data and whether anything renders at all
are the module's. The moment core types a slot by its content it has re-acquired
the game semantics phase 3 exists to remove.
This is the one place the client registry is not fail-open. An unknown slot, a
non-component and a second fill all throw, matching checkExtensionShape
server-side, because a dropped nav row costs a link the viewer can reach another
way while a silently dropped extension is invisible to everyone including its
author. A throw is always a programming error and never a race: core declares in
its own bundle and every module chunk is a deferred script injected after it.
Reading stays fail-safe -- undeclared and unfilled both read null -- and a
filling component renders inside an error boundary. That asymmetry is where the
client differs from the server: a module route that throws costs the module's own
page, but an extension throws inside CORE's, and the whole reason core keeps
ownership of that page is that it stays usable.
Core decorates a slot through <Slot wrap>, not by asking whether it is filled.
The obvious alternative is right about the unfilled case and wrong about the
failed one -- the extension is filled, so the separator renders, and then the
component throws into the boundary and leaves the separator behind on its own.
wrap puts core's decoration inside the boundary where it shares the extension's
fate. Found in a browser, with the footer's separator, which is the only place
either could have been found.
MODULE_API_VERSION 1.1.0 -> 1.2.0, both halves: the two state ONE version.
Contract: docs/website/MODULE_API.md 3.7.
Co-Authored-By: Claude <noreply@anthropic.com>
25 of 82 test files left with the module. Three that core keeps needed splitting
rather than moving, and the split is the boundary in each case.
announceJobs.test.js keeps the announce PIPELINE -- the shared backoff schedule,
the parent-status rollup, core's Discord leg -- and loses the town-crier text
building and classification, which are a module's leg. pushDispatch.test.js
keeps the SSRF guard and publish() delivering a content-free tickle, and loses
mapShardEvent and the shard fan-out, which are a module's catalog.
playerRouteAccess.test.js is the one worth explaining. It guards a real past bug
-- an admin 403'd off their own characters -- and it did so through
/player/shard/accounts, which is now module-owned. The guarantee it protects is
CORE's, though: /player/* is role-agnostic self-service, staff are a superset of
players. So it stays here and asserts that through /player/appeals, a core route
with the same gate. Moving it would have left core with no test of its own tier
rule, which is precisely what regressed once before.
The remaining updates are core's own tests catching up: ctx has four more
members, registerCore now registers only what core owns (one stream, one leg, no
filled slot), and the extension-slot test asks for the DECLARED slot's router
rather than the filled one, since core declares it and a module fills it. The
gated-surface floor drops from >100 to >50 -- it is there so a filter matching
nothing fails loudly, not to track core's exact route count.
616 core tests and 160 client tests pass; the module's own suite is 351.
Co-Authored-By: Claude <noreply@anthropic.com>
40 files, ~9,674 lines, 27 of 68 tables. Core no longer contains anything that
knows what a shard is.
BREAKING for a deployment only in the sense that the module must be installed
for these URLs to answer -- no URL moved. routes.manifest.json goes 228 -> 158
public routes here, and the 70 that left reappear byte-identical when the module
is loaded: verified by generating the manifest against core+module and diffing
it against the pre-extraction file. Zero missing, zero added, and routes.guards
identical across all 228, so no auth gate moved either.
The five tier mounts are gone from public/admin/player index.js and are still
served: the loader mounts them onto the same routers after every core mount.
That ordering is also what keeps the prefixes unclaimable -- the collision check
asks the live router what core owns, so a second module claiming /shard is
rejected against the mounts actually present rather than against a list.
server.js loses its eight UO call sites to the module's onBoot/onShutdown.
schema.sql loses its 27 shard_*/uo_link_* statements; the two that FK into users
are why the fragment replays AFTER core's schema, and no core table ever
referenced a module table, which is what makes core still able to boot alone.
Verified against a running server with the module installed: it loads, mounts
five prefixes, replays 35 statements, warms up and reaches `started`; public
shard and atlas routes answer 200 with real data (800 creatures, 6,455
spawners), admin and player answer 401 from core's tier gates, and the extension
slot answers at /admin/users/:id/shard/*. Core's own SPA renders the shard and
atlas pages unchanged against the module-served API, with no console errors and
no CSP reports.
Co-Authored-By: Claude <noreply@anthropic.com>
Everything the extraction needed from core that ctx did not already offer.
Additions only, so minor.
ctx.activity.log, because an admin action a module performs has to land in
core's one audit log or the trail has a hole exactly where a module operates the
game -- a module keeping its own log would be a second place to look, which in
practice means a place nobody looks. Write-only; reading the log is the admin
panel's job and it spans every actor.
ctx.users.getById, one function for one caller: the admin.users.detail slot
router needs the user its prefix names. ctx.site.baseUrl, because a module has
to build absolute links and §2.7 forbids it reading core's APP_BASE_URL -- a
getter, not a captured string, so it cannot go stale against the env.
ctx.middleware.rateLimit is core's makeLimiter, plus accountChangeLimiter handed
over whole. The split is deliberate: a module states its own window and cap
because it knows what its endpoints cost, and takes the plumbing from core so
there is one express-rate-limit in the process and one place a breach is logged.
accountChangeLimiter is shared policy -- core's /auth/me and /player/account sit
behind the same counter -- so a module's account-change route has to land IN it
rather than beside it. marketLimiter was UO policy living in core's file and
leaves with the route it guards.
registerPostHook is the fourth registry, and the last thing binding core to the
module. Core's post controller called newsGump.syncPost directly: core's CMS
naming a UO file. It now publishes what it already knows and a subscriber
decides what to do with it. Not folded into registerAnnounceLeg, which fires on
the same transition, because a leg is a one-shot DELIVERY with retry and
classification while a post hook maintains idempotent STATE, runs on delete as
well as save, and refreshes silently on an edit.
Also fixes a real loader defect the extraction exposed: schema table names were
matched against the RAW file, so a fragment whose header says "every CREATE
TABLE carries IF NOT EXISTS" was rejected for a prefix violation on a table
called `carries`. module-uo's fragment hit exactly that. Both scans now read
split statements, which strip comments -- the same class of bug as a boundary
check failing on its own documentation.
Co-Authored-By: Claude <noreply@anthropic.com>
Closes Phase 2. Modules live on a mount, never in the image — that is what
lets an operator add one to a pull-only deployment without building anything.
`./modules` is a bind mount rather than a named volume: placing a module
directory by hand is a supported install (MODULE_SYSTEM.md §2.5), and that has
to be doable from the host rather than through `docker cp`. Read-write, because
the admin panel's install/uninstall unpacks and removes directories there.
The directory is tracked via its README so it exists in the checkout with the
operator's own ownership — Docker recreates a missing bind-mount source as
root:root, which the container user could not then write. `.dockerignore`
excludes it so a module in the builder's working tree can never ship inside an
image.
Also corrects the route-manifest generator's list of filesystem-conditional
mounts, which never picked up `/modules` when PR 7 added it. Comment only; the
generator filters on an allowlist, so its behaviour was already right.
Verified against a real container, not just a parsed compose file: image
carries an empty node-owned /app/modules despite a module in the build context;
a module on the bind mount loads, mounts, replays and reaches `started`;
`/api/v1/public/modules` lists it; the chunk serves from the entry's directory
only (server source and module.json 404) with `no-cache`; the injected tag
follows core's bundle; and in Chrome the page renders on first paint inside
core's PublicLayout with its nav row interleaved into core's public nav, under
enforced `script-src 'self'` with zero CSP reports and no console errors.
Removing the directory by hand reconciles the row to `startup_failed`/`require`
and leaves core healthy with no injection.
933 server + 160 client tests pass, manifest unchanged at 230 routes, swagger
regenerates byte-identical.
Co-Authored-By: Claude <noreply@anthropic.com>
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>
Phase 2, PR 7 of docs/website/MODULE_SYSTEM.md 2.7 — the client half's
delivery. A module's prebuilt chunk is served, injected, handed core's React
and its UI kit, and its routes are rendered by App.jsx. The registry is empty
on a bare core, so nothing an operator can see changes.
Client:
- modules/registry.js — registerRoutes/registerNav/registerFeatureProvider,
with the URL namespace written by core, never by the module
- modules/shared.js — window.__rg: React, react-dom/client, react-router-dom,
react/jsx-runtime, the registry, the seven-member UI kit and the request
primitive, frozen
- App.jsx reads routesFor for all three areas; nav consumption is PR 8
- main.jsx publishes the global, then mounts on DOMContentLoaded
Server:
- the loader validates client.entry and publishes clientChunks() and
clientEntryUrls(); an entry in the module root is rejected, because the
directory it sits in is what gets served
- app.js mounts each chunk at /modules/<id>/ behind the module's state guard
with no-cache; anything else under /modules is a 404, not the SPA shell
- htmlShell injects the tag before </body>, so core's bundle runs first
wherever a bundler puts it
Found by loading a real chunk in a browser, and fixed here: core mounted before
any module chunk had evaluated, because document.readyState during a deferred
script is 'interactive', not 'loading'. Every test passed against that build.
The smoke is written down in MODULE_API.md 7.7.
933 server tests (+23), 123 client tests (+14). routes.manifest.json unchanged
at 230 routes; the OpenAPI spec regenerates byte-identical.
Co-Authored-By: Claude <noreply@anthropic.com>
Phase 2, PR 6 of docs/website/MODULE_SYSTEM.md 2.7 — the first module-system
URL a client can see. The SPA and the Android app feature-detect against the
capabilities a module declares; the shape is settled in MODULE_API.md 2.9.
Four decisions, and what is absent from the payload is most of the design:
* started modules only. A module that is disabled or failed to load is
ABSENT, exactly as 4.4 already leaves its routes and its nav absent, so a
client renders a site without that capability rather than advertising one
that 503s.
* no state, failure_stage or failure_reason. Where a module broke belongs to
the admin Modules screen, and the reason is an exception string from inside
core — not anonymous-visitor business.
* no client chunk URL. htmlShell injects a script tag per started module
(3.1.3), so the browser is handed the tag rather than a URL to fetch. This
endpoint feature-detects; it does not load. MODULE_SYSTEM 2.6 step 4 is
amended to match (API 6.7).
* no siteMode gate and no database — the same class as /public/status and
/public/version, so a client can still feature-detect during maintenance.
It is a capability router of its own rather than a fifth singleton in
site.router.js, and that is load-bearing: the loader's prefix-collision probe
reads the live tier stack and skips root-mounted layers, because a use('/', ...)
matches every path. A route inside the root-mounted site router would be
invisible to it — mounting use('/modules', ...) is what makes "no module may
claim /modules" a rule the loader enforces.
910 tests pass (+9, every one on the boundary — what must NOT appear).
routes.manifest.json gains exactly the one route and routes.guards.json records
it with an empty gates list, which is itself the assertion that it is ungated.
Co-Authored-By: Claude <noreply@anthropic.com>
`npm test` never terminated. Twenty-two test files omitted the two lines that
point the pool at a dead port, so utils/db.js -- which builds its mariadb pool at
require time and calls dotenv.config() itself -- picked up server/.env and opened
five live connections to the developer's MariaDB. The tests still passed, because
they stub their models and never issue a query; the only symptoms were a process
that never exited and five connections held for as long as it lived. Thirty
stranded workers is 150 connections, which is the whole server's limit, and that
is the "too many connections" this workspace has hit before.
The convention was right and only ever as good as the next test file's memory of
it, so it moves into the harness: test/_setup.js is loaded with --require by the
npm script, ahead of the test file it hosts, which is the only moment early
enough to matter. It pins the dead port -- dotenv does not overwrite an existing
variable, so an explicit DB_PORT= still wins for anyone who wants a live database
-- and closes the pool after the file's tests, so the process exits at once
instead of waiting out the driver's connect retries. The per-file preambles stay:
they keep `node --test test/one.test.js` safe on its own.
Two supporting fixes:
- db.close() is idempotent. pool.end() throws "pool is already closed" on a
second call, and closing twice is now normal rather than exceptional -- the
harness closes the pool for every file on top of the suites that close it
themselves, and a SIGINT followed by a SIGTERM already reached the shutdown
handler twice.
- test/_helper.js's close() destroys open connections. server.close() only stops
accepting and waits for existing connections to end, and node's global fetch
keeps its sockets alive, so the listener outlived the test that created it --
invisible until now, because the pool was holding the process open anyway.
announceJobs.test.js alone: 120s+ hang -> 0.35s. The whole suite now finishes in
~75s where it previously did not finish at all: 901 tests, 901 pass, verified
three times on CI's exact platform (node:20 on Linux, via Docker).
Co-Authored-By: Claude <noreply@anthropic.com>
Phase 2, PR 5 of docs/website/MODULE_SYSTEM.md 2.7. api.onBoot/api.onShutdown
stop throwing, server.js gains one call on each side, and the 2.4 state machine
finally runs against real outcomes -- which is what makes 4.5's `disabled` 404
leg reachable for the first time.
Dispatch and reconcile live in src/modules/lifecycle.js rather than in the
loader, for the reason the schema replay does: routeManifest.js and swagger.js
both require app.js against a dead pool, so the loader may not reach the
database. The two halves meet at exactly one function, loader.setState(), so the
in-memory record the dispatch guard reads and the row the admin panel reads are
moved together and cannot disagree.
Four decisions, all recorded in MODULE_API.md 2.5 and 4.4:
- The loader classifies its failures by 4.3 step, so failure_stage says where a
module broke instead of being a column nothing ever filled. The four steps
readManifest covers in one pass label themselves; the rest are inferred from
how far load() had got, and an unlabelled throw is recorded against the step
that was running rather than guessed at.
- A row whose directory is gone is marked startup_failed rather than left
claiming `enabled` -- the boot reset has just moved it there, and a row
claiming to be enabled for a module that is not on the volume is the one state
that is simply untrue. An uninstall leaves `disabled`, which the reset never
touches, so this catches only a hand-deleted directory.
- Core's eight UO boot call sites stay in server.js until Phase 3. Unlike a
registered announce leg, a boot call site already has somewhere to live, so
moving it now would be extraction done early in a phase whose exit criterion
is that nothing changes.
- onBoot gets no timeout. Shutdown races a SIGKILL and boot does not, and a slow
onBoot delaying the listener is the contract's promise to a module that must
warm up before it serves.
The operator's switch wins over everything: a disabled module is guarded, not
booted, and does not have its failure re-recorded, or an outcome would silently
switch it back on next boot. Every database write in the reconcile is
individually caught -- a row that will not update is worse reporting, never a
failed boot.
900 tests pass (17 new). routes.manifest.json is unchanged at 229 routes and the
OpenAPI spec regenerates byte-identical.
Co-Authored-By: Claude <noreply@anthropic.com>
Phase 2 PR 4 of docs/website/MODULE_SYSTEM.md §2.7. Adds server/src/modules/registries.js
and moves core's own notification streams, announce leg and users-detail routes
behind it, so the three seams §1.8 and §1.9 named are exercised on every boot
before any module depends on them.
Registering is validate-then-commit per registrant: the loader stages what a
module claims and the second pass commits it, so a module that throws halfway
through register() — or fails checkDeclared after it — leaves nothing behind.
That is the registry-side twin of PR 2's second-pass mount rule.
Four decisions, all the recommended option:
- announce legs became a child table. `announce_job_legs` replaces the
towncrier_*/discord_* column groups, so the leg set is data: core registers
`discord`, module-uo will register `towncrier`, and a module cannot ALTER a
core table to add its own. Backfill is guarded on information_schema (a
SELECT of a dropped column is a parse error, not a runtime one) and the
columns go with DROP COLUMN IF EXISTS. Verified against the live dev DB:
three legacy jobs migrated faithfully, three replays, no duplicates.
- `mapEvent` dropped from registerNotificationStreams. §1.8 already inverts the
push path so a module owns fromShardEvent and calls core's publish() with a
stream id it resolved; a second mapping mechanism was a leftover. The public
safety filter, the kinds it reads and the streams it protects now live in one
file and move together.
- core registers through the same staging area a module uses, via an explicit
registries.registerCore() in app.js before modules.load().
- core's six /admin/users/:id/shard/* paths now go through the
`admin.users.detail` slot, and getUser moved back to admin.controller.js.
Found on the way, and the reason two build tools changed:
- scripts/routeManifest.js could not decode a parameterised mount. Its
unwinder expected `(?:([^\/]+?))`; express 4.22 emits `(?:\/([^/]+?))` with
the separator inside the group. The branch had never run. It threw rather
than guessing, which is what it is for.
- swagger-autogen cannot follow a route into an extension slot — the slot's
router is created by declareSlot() and filled later, so there is no literal
mount for a static parse. Regenerating deleted 407 lines and printed
`Swagger-autogen: Success`, the spike's exact failure (MODULE_API.md §7.4).
swagger/slotSpecs.js generates a fragment per filled slot and re-roots it at
the prefix the router actually hangs at in the live app — read from the
express stack via routeManifest's own mountPath, so the manifest and the spec
cannot disagree. swagger/mergeSpec.js is the merge helper core owes for
module fragments anyway (§6.1a), proved here against core's own slot first.
884 tests pass (856 before). routes.manifest.json is unchanged at 229 routes.
The OpenAPI spec diff is two lines of intent: the retry endpoint's summary, and
its `leg` no longer being a fixed enum.
Co-Authored-By: Claude <noreply@anthropic.com>
Phase 2, PR 3 of docs/website/MODULE_SYSTEM.md 2.7. ensureSchema() now replays
every installed module's schema fragment immediately after core's schema.sql,
per MODULE_API.md 2.6.
The work splits across two files on the line of whether a database is needed to
know the answer. loader.js VALIDATES a fragment at load time, before anything is
mounted, because every rule 2.6 states about the SQL is knowable by reading it;
a module that breaks one never mounts (4.4, left column). modules/schema.js
EXECUTES it, so the only failures there are the ones the database alone could
report, and those are post-mount and answer 503 (4.4, right column).
Validation is a leading-verb allowlist -- CREATE, ALTER, INSERT, UPDATE, the
four core's own schema.sql uses -- rather than the DROP denylist 2.6 words it
as. A fragment is replayed on every boot, so TRUNCATE and DELETE would empty a
table at each restart and RENAME would fail at the second one; a denylist only
ever bans what somebody thought of. A CREATE TABLE missing IF NOT EXISTS is
rejected for the same reason: it works once and fails every boot after, which
presents to an operator as a module that broke on restart.
The splitter moves to utils/sqlStatements.js so core's schema and a fragment are
split by literally the same code, which is what 2.6 promises. It is its own file
rather than an export of utils/db.js because the loader validates fragments at
require time and must not drag the mariadb pool into app.js's require chain.
The replay sits outside ensureSchema's wait-for-the-database retry loop: a
fragment that throws is one module's failure, not a signal the database is
coming up, and retrying core's whole schema nine more times over one module's
bad SQL would turn a 503'd module into a two-minute boot.
Found while wiring it: db/seed.js calls ensureSchema() standalone for
`npm run seed`, without ever requiring app.js, so the loader has not scanned and
fragments()'s 7.6 throw would have broken seeding outright. The replay asks
isLoaded() and logs the skip rather than swallowing it -- a booting server
quietly getting no module tables is the thing 7.6 exists to prevent.
Verification:
- 856 server tests pass, 14 new. moduleSchema.test.js injects the query fn, so
the exact statements and their order are asserted with the pool at a dead port
like every other suite.
- routes.manifest.json and routes.guards.json diffs are zero lines, 229 routes
-- the phase 2 exit criterion. swagger-output.json regenerates byte-identical.
- Run for real against the local MariaDB with two fixture modules: a good
fragment created its table, applied its ALTER and seeded its row; a fragment
whose SQL passes validation but the server rejects (`id NOTATYPE`) marked only
that module startup_failed, its route answering 503 while the other answered
200; a second ensureSchema on the same database was a clean no-op.
Co-Authored-By: Claude <noreply@anthropic.com>
Phase 2 PR 2 of docs/website/MODULE_SYSTEM.md 2.7. Adds
server/src/modules/{loader,semver,version}.js: the synchronous scan of
MODULES_DIR, manifest validation, prefix and table-name collision
rejection, per-module try/catch and the mount into the three tier
routers behind the MODULE_API.md 4.5 dispatch guard.
Two decisions the contract left open, both now written up there:
- The load trigger is one explicit modules.load(tierRouters) call in
app.js, not a lazy scan (API 7.6). Accessors throw until it has run,
because "no modules installed" is a real answer a caller must not be
handed by accident.
- Whether core owns a prefix is asked of the live tier routers via
express's own layer.match(), skipping root-mounted layers, rather than
a hardcoded table -- the spike's was already stale when written
(API 4.3).
Mounting is a second pass after every module is validated. Doing it
inside the scan loop makes the first module's layers indistinguishable
from core's, so the second module claiming a taken prefix is told it
collided with core and the module-versus-module check is unreachable.
registerExtension/NotificationStreams/AnnounceLeg and onBoot/onShutdown
throw "not available until phase 2 PR 4/5" rather than no-op; an
accepting stub would let a module believe it had registered something.
No schema replay, no boot dispatch, no installed_modules reconcile --
those are PRs 3 and 5, and until PR 5 a record's state is in memory only.
No module ships on the volume, so nothing an operator or client can see
changes: 842 tests pass, routes.manifest.json is unchanged at 229 routes
and swagger-output.json regenerates byte-identical.
Co-Authored-By: Claude <noreply@anthropic.com>
Phase 2 PR 1 of the module system (docs/website/MODULE_SYSTEM.md 2.7). The
table and the state machine only: no loader, no routes, no boot wiring, so
nothing an operator or a client can see changes and the route manifest diff
is zero lines.
The five states of 2.4 live in one `state` column: installed -> enabled ->
started, with disabled and startup_failed as the recoverable ones. The row is
a record of what happened, never the source of truth for what is mounted --
the loader scans the filesystem at require time, before the database is
reachable (MODULE_API.md 4.1), which is what keeps routes.manifest.json
generatable against a dead database.
Two rules the model owns and the boot path will lean on:
- Every boot resets each non-disabled row to `enabled` and clears its
recorded failure, so a startup_failed module is retried on the next restart
and a fixed one recovers with no admin-panel visit. `disabled` is the one
operator decision rather than outcome, so it survives untouched -- and a
disabled module's failure is a no-op, never a re-enable.
- A failure carries the stage it happened at, and every non-failing
transition clears it, so a running module can never show a stale reason.
An illegal move throws instead of writing a row that misrepresents the state,
except on the two boot-path softenings noted above, because one module's
failure must never become everybody's.
22 model tests over an in-memory fake; the SQL and the DDL were round-tripped
against a real MariaDB separately.
Co-Authored-By: Claude <noreply@anthropic.com>
Long workstreams land phase by phase on `edge` and reach `main` as a single
cutover. With `branches: [main]` alone, every one of those phase PRs merges
with no checks at all -- no server tests, no client build, no bot install --
and the whole workstream runs blind until the cutover, where the breakage
arrives all at once and un-bisected.
This is not hypothetical: it is what happened to all nine Android M12 phase
PRs in the Android-app repo, whose pr-checks.yml carries the same trigger.
It matters for the module system specifically because Phase 2's exit
criterion IS a CI result -- a zero-line routes.manifest.json diff and a
passing suite -- and that manifest is the frozen URL surface protecting
three shipped clients. A branch accumulating work for weeks needs the gate
more than main does, not less.
Landing before the module-system edge branch is cut, so the first phase PR
is checked. build-images.yml is deliberately untouched: it triggers on push
to main, so images publish and production rolls at the cutover and never
before.
Co-Authored-By: Claude <noreply@anthropic.com>
The public header's dropdown trigger is a <button class="pill"> sitting in a row
of <a class="pill"> links, and it rendered ~7px shorter.
It was not failing to pick up the theme: font-size, font-family, padding, border
and box-sizing all matched exactly. The one property that differed was
line-height, because form controls do not inherit it — the UA stylesheet gives
<button> `line-height: normal` (~1.15), while the anchors inherited body's 1.6.
38.02px against 31px, which is precisely 22.016 - 15.8.
Stating it on .pill fixes it at the source rather than patching the one button:
every other property in that rule is already explicit for the same reason, and
this was the remaining gap. The value matches body's 1.6, so no link pill
changes. The ~70 <button class="pill"> elsewhere in the admin gain the same 7px
and now line up with the .btn buttons they sit beside.
.btn has the same latent difference and is deliberately left alone: it is used on
80 buttons and 2 anchors, they never appear on the same row, so nothing is
visibly wrong and the blast radius is not worth it.
Co-Authored-By: Claude <noreply@anthropic.com>
Phase 10 of docs/website/THEMING_AND_NAV.md, asked for before the edge -> main
cutover. An admin can now create dropdown sections in the public header, organise
the coded entries into them, and add links of their own.
This deliberately amends §7, which said the override layer "cannot introduce a
`to` that is not already in the hardcoded NAV array". That stays true of every
CODED entry; an admin may now also add a link, restricted to a same-origin path —
no scheme, no protocol-relative //host. A link carries no gate of its own and
needs none: the page behind it enforces its own access, so an added link
advertises a route and never grants one.
The invariant is kept structurally rather than by vigilance. Coded entries live
in an `items` map whose keys must be routes the base array declares, so that map
cannot invent a route; everything that CAN name an arbitrary path lives in
`links`, which is the one place the path rule is applied — on both the write and
the read path.
nav_public therefore grew a { items, sections, links } wrapper. A bare map still
reads as the items map, and a nav with no sections still stores one, so this
changed nothing for a nav that does not use it. Free to do now because nothing
has shipped; after the cutover it would have needed a migration.
The Public tab gets its own editor. A public section is an entry in the
top-level order that the admin created and can drag among the pills, unlike the
admin sidebar's four coded sections, where only membership moves — that is a tree
rather than a list of groups. Deleting a section returns its entries to the top
level rather than removing them, which is the one destructive act this screen
could otherwise commit.
The dropdown opens on click and never on hover, and its trigger is not a link: a
hover menu is unusable on touch, and a trigger that navigates means tapping to
open takes you somewhere instead. Escape closes and returns focus, an outside
press closes, navigating closes, and Arrow Up/Down walk the items.
pruneNav applies the shard-feature gate inside a section and drops one it leaves
empty, so a dropdown never opens onto nothing.
Also fixes a bug this surfaced in the phase 6-8 code: the save path judged "does
this route still exist?" against the palette — the base array already filtered to
what the editing admin can see — so on the public header a feature-gated row's
override could never be carried through and would have been silently reset.
Membership is now judged against the full coded nav while the rows still come
from the palette.
Co-Authored-By: Claude <noreply@anthropic.com>
