Merge pull request 'docs(website): record split PR 1 — admin users, account, invites, auth providers' (#53) from docs/admin-router-split-1 into main

Reviewed-on: #53

website/API_V2_PLAN.md

@@ -1,6 +1,7 @@
 # Website API — router domain split + CSP hardening
 
-Status: **planning** · Target repo: `website/` · Docs owner: this file + `BACKEND_DESIGN.md`
+Status: **in progress** — PR 0 (route manifest), CSP report-only, and split PR 1 of 5 have landed ·
+Target repo: `website/` · Docs owner: this file + `BACKEND_DESIGN.md`
 
 > **This file replaces the earlier "API v2" plan** (auth merge → CSP → domain split, with a parallel
 > `/api/v2` mount and an `/api/mobile` facade). Three of those four pieces are **not being built**:
@@ -303,6 +304,63 @@ Because the auth model is untouched and the URLs are frozen, each PR is a **pure
 with a green test suite and a zero-diff route manifest as its acceptance criteria** — which is what
 makes grouped PRs actually reviewable.
 
+> **Step 6 correction:** `PROJECT_TREE.md` is no longer hand-edited. Since website#98 it is
+> auto-generated by the `sync-project-tree` CI workflow, which opens its own docs PR after a merge to
+> `main`. Leave it alone in split PRs. `BACKEND_DESIGN.md` §2/§4 are still manual.
+
+### PR 1 — as landed
+
+`admin/index.js` owns the shared `noindex, isLoggedIn, staffOnly` gate and the mount table, and
+declares no routes itself. The gate sits **ahead of every mount**, so a capability router extracted in
+a later PR cannot silently ship without it. Route counts:
+
+| Router | Routes | Prefix | Extra gate |
+|---|---|---|---|
+| `account.router.js` | 6 | `/admin/account` | none — self-service, an editor manages their own 2FA |
+| `users.router.js` | 15 | `/admin/users` | `adminOnly` at router level |
+| `invites.router.js` | 3 | `/admin/invites` | `adminOnly` per route |
+| `authProviders.router.js` | 4 | `/admin/auth` (routes are `/providers[/:id]`) | `adminOnly` per route |
+| `admin.routes.js` (residual) | 82 | group root, mounted last | unchanged |
+
+6 + 15 + 3 + 4 + 82 = the 110 inventoried admin routes. None of the four prefixes appears in the
+residual file, so nothing depends on mount ordering.
+
+Three findings worth carrying into PRs 2–5:
+
+- **The self-service `/shard/*` routes stay with `shard` (PR 4), despite their `Admin · Account`
+  swagger tag.** The invariant is *prefix ownership*, not tag agreement: one router owns `/shard`, so
+  splitting six routes off it by capability would mean two routers mounting under the same prefix and
+  an ordering hazard for no gain. Retag them in PR 4 if the tag still grates.
+- **`usersRouter.use(adminOnly)` is exactly equivalent to the old `adminRouter.use('/users', adminOnly)`**
+  now that the router is mounted at `/users` — but only because it is mounted at a prefix. Under a
+  *pathless* mount, a bare `use(gate)` would run for every request passing through en route to a later
+  mount, 403-ing an editor on `/admin/posts`. Do not "simplify" a prefix mount away.
+- **`routes.guards.json` came back zero-diff too**, not just the manifest — no route lost or gained a
+  gate. Worth checking both every time; the guards file is the one that would catch a dropped
+  `adminOnly` that the method+path freeze cannot see.
+
+The OpenAPI spec was also byte-for-byte unchanged, which required a prerequisite fix — see below.
+
+### The swagger path-normalization prerequisite (landed before PR 1)
+
+swagger-autogen builds a path by string-concatenating the mount prefix with the route argument, so a
+capability router mounted at `/users` whose collection route is `router.get('/')` documents as
+`/api/v1/admin/users/` — advertising a URL no client calls while dropping the one the SPA, the Android
+app and the Discord bot all do. Express is indifferent; the published spec is not. It also emits path
+keys in *router-traversal order*, so moving a route between files rewrote most of the ~5k-line
+committed artifact even when the API was provably unchanged — burying the one line a reviewer needs.
+
+Both are fixed once in `server/swagger/swagger.js`, which post-processes the generator's output to
+strip trailing slashes and sort path keys (throwing on a collision rather than silently dropping an
+operation). It shipped as its own PR ahead of PR 1, verified inert by the regenerated spec being
+byte-for-byte the sorted form of the previously committed one — 198 operations, none added or removed.
+`#swagger.path` was rejected as the fix: it bypasses the mount prefix, so every route would hardcode
+its absolute path in a comment that silently lies the moment a mount moves.
+
+**Consequence for PRs 2–5: the swagger diff is now a signal.** With sorting in place, a pure route
+move produces *no* spec diff at all, so any diff there means an annotation actually changed. Treat
+`swagger-output.json`, `routes.manifest.json` and `routes.guards.json` as three zero-diff gates.
+
 ### PR 0 — the route manifest (prerequisite of the first split PR) — **landed**
 
 > **Status: shipped.** `server/scripts/routeManifest.js` + `npm run routes:manifest`,
@@ -390,7 +448,7 @@ deliberate `+1` in the manifest — which is exactly the mechanism working as de
    data**, not on code: watch the `csp` log tag for `frame-ancestors` reports across one release before
    flipping. Also decide there whether `/api/csp-report` is retired with the report-only twin or kept
    as a `report-to` group on the enforced policy.
-4. **PR 1 — admin:** `users`, `account`, `invites`, `auth` (providers).
+4. **PR 1 — admin:** `users`, `account`, `invites`, `auth` (providers). ✅ landed
 5. **PR 2 — admin:** `moderation`, `bot-activity`, `activity`.
 6. **PR 3 — admin (content):** `posts`, `pages`, `wiki`, `uploads`.
 7. **PR 4 — admin (ops/config):** `shard`, `uo-link`, `email`, `discord-bot`, `settings`, `site-mode`,

website/BACKEND_DESIGN.md

@@ -29,11 +29,15 @@ Public contact email: **UOMysticmoon@gmail.com**
 
 Skeleton from the spec, with a small number of justified additions marked **(+)**.
 
-> **Planned change:** the monolithic route files below (`admin.routes.js` especially, 1552 lines /
-> 110 routes) are being split into one router file per business capability — **in place, with every
-> URL unchanged**. This section and §4 get updated as each split PR lands. See
+> **In progress:** the monolithic route files below (`admin.routes.js` especially, originally 1552
+> lines / 110 routes) are being split into one router file per business capability — **in place, with
+> every URL unchanged**. This section and §4 get updated as each split PR lands. See
 > [API_V2_PLAN.md](./API_V2_PLAN.md) § Phase 2.
 >
+> **Landed so far:** admin `users`, `account`, `invites` and `auth/providers` (28 routes) now live in
+> their own routers under `admin/`, behind a new `admin/index.js`. The remaining 82 admin routes are
+> still in `admin.routes.js`, and `public/` and `player/` are untouched.
+>
 > "Every URL unchanged" is enforced mechanically, not by review: `server/scripts/routeManifest.js`
 > (`npm run routes:manifest`) walks the live Express stack and writes the sorted
 > `{ method, path }` freeze to `server/routes.manifest.json`, mirrored here as
@@ -56,7 +60,20 @@ server/
         v1.router.js            mounts /auth /public /admin
         auth/    auth.routes.js    + auth.controller.js
         public/  public.routes.js  + public.controller.js
-        admin/   admin.routes.js   + admin.controller.js
+        admin/   index.js          mounts the capability routers below at their
+                                   own prefixes; owns the shared
+                                   `noindex, isLoggedIn, staffOnly` gate and
+                                   declares no routes itself
+                 account.router.js       (6)  /admin/account  — self-service, no adminOnly
+                 users.router.js        (15)  /admin/users    — adminOnly
+                 invites.router.js       (3)  /admin/invites  — adminOnly
+                 authProviders.router.js (4)  /admin/auth     — adminOnly
+                 admin.routes.js        (82)  everything not yet split, mounted
+                                              last at the group root; goes away
+                                              when the final split PR lands
+                 admin.controller.js + the per-capability controllers
+                                   (already domain-split; the split PRs re-wire
+                                   routes, not logic)
     model/
       users/     users.model.js    + users.db.js
       posts/     posts.model.js     + posts.db.js     (news/five-on-friday/newsletter/screenshots)
@@ -440,7 +457,12 @@ expiry (~5 min); `/exchange` is rate-limited per-IP. The bridge tables self-prun
 
 Public content GETs pass through the **siteMode** gate (§5).
 
-### /admin  (admin.routes.js → admin.controller.js) — all behind `isLoggedIn` + `noindex`
+### /admin  (admin/index.js → the capability routers in §2) — all behind `isLoggedIn` + `noindex` + `staffOnly`
+
+`admin/index.js` applies the shared gate and mounts each capability router at the prefix it owns;
+`users`, `invites` and `auth/providers` add `adminOnly` on top. Routes not yet extracted still live
+in `admin.routes.js`, mounted last at the group root. The URLs below are unaffected by which file a
+route currently sits in — that is the property the route manifest freezes.
 | Method | Path | Purpose |
 |---|---|---|
 | GET | `/dashboard` | current mode, last change time + who, content counts, recent activity |