diff --git a/book/01-first-module.md b/book/01-first-module.md index 3a26408..42840f1 100644 --- a/book/01-first-module.md +++ b/book/01-first-module.md @@ -47,6 +47,30 @@ cp -r template/ ~/my-module cd ~/my-module ``` +**Run every check on the untouched copy before you change a line.** Jump ahead to +*Build it* and run all of it — the tests, the build, the three guards — on the +template exactly as it arrived: + +```bash +npm ci --prefix server && npm test --prefix server +npm run check:imports --prefix server +npm run check:swagger --prefix server +npm ci --prefix client && npm run build --prefix client +npm run check:externals --prefix client && npm test --prefix client +``` + +It takes two minutes and it buys you a **baseline**. Every one of those commands +is green on a pristine template, so from here on a red one is something you did — +and you will know which edit did it, because you were green a moment ago. Without +that, the first failure is ambiguous forever: is this my mistake, or was the +template already like this? + +That is not a hypothetical. The kit's own acceptance run +([`kit-acceptance.md`][acceptance]) found `check:swagger` failing on an untouched +copy on Windows, with a message that blamed the reader's routes. It is fixed, and +the reason the run *found* it rather than being derailed by it is that it had a +baseline. + Your module id is the single most load-bearing string in it: it is the directory core loads you from, the key in core's database, the URL segment every one of your pages hangs under, and the prefix every one of your tables must carry. It must @@ -242,3 +266,4 @@ module must never do. [api]: https://gitea.whitlocktech.com/RunicGateway/docs/src/branch/main/website/MODULE_API.md [issues]: https://gitea.whitlocktech.com/RunicGateway/Integration-kit/issues [renamecheck]: ../scripts/checkRenameSites.js +[acceptance]: https://gitea.whitlocktech.com/RunicGateway/docs/src/branch/main/modules/kit-acceptance.md diff --git a/book/02-website-module.md b/book/02-website-module.md index 51d9d3c..91a1345 100644 --- a/book/02-website-module.md +++ b/book/02-website-module.md @@ -392,12 +392,34 @@ and empty states, the async hook every data page uses, and read-only access to t session and site settings. Enough to build a page that looks like the site it is installed in, and nothing else. +**`PublicLayout` needs a `shell`, and this is the one that will catch you.** The +layout is the *chrome* — header, footer, the flex column they sit in. The `shell` +prop is the *body*: the centred max-width column, the vertical padding, and the +element whose `flex: 1` is the only thing holding the footer at the bottom of the +viewport. + +```jsx + // 'narrow' · 'mid' · 'wide' +``` + +Omit it and your content starts hard against the left edge of the window with no +padding, and the footer climbs up underneath it. It reads as a stylesheet bug in +your module and it is not one — core's own pages write that wrapper by hand, and +before `MODULE_API_VERSION` 1.5.0 a module had no way to. **Name a width, never a +class:** the class names are core's stylesheet's and it is free to rename them, +which is exactly why they are not in the contract and this prop is. + +That paragraph exists because the kit's acceptance run +([`kit-acceptance.md`][acceptance]) built a module by following this chapter to the +letter, and its page rendered outside the site. Everything else it wrote was right. + **It is curated and closed, not a re-export of core's component library.** Adding -to it is a minor version bump; changing a kit component's props is a major one. +to it is a minor version bump, and so is adding an optional prop to a member; +changing an existing prop is a major one. That is a real constraint on core, and it is the price of the boundary being worth anything. -So: when you want an eighth thing, bundle it. Tables, chips, tabs, editors — those +So: when you want something it does not have, bundle it. Tables, chips, tabs, editors — those are yours, and your chunk carries them. Reaching into core's tree for a component is the one thing that is never available, and `template/server/scripts/checkImports.js` exists to make sure a moment of weakness fails the build instead of shipping. @@ -433,10 +455,26 @@ nothing at runtime will ever say so. The trap: **swagger-autogen reports a broken annotation and then prints `Success`.** It logs a syntax error, drops that annotation, and exits zero. The template's generator captures those diagnostics and fails on them — keep that. -Two ways an annotation breaks are an object literal one brace short, and a `"` or -a backtick inside a single-quoted description. A third is only visible in a -rendered page: an escaped apostrophe (`\'`) survives literally into the output, -because the annotation is never evaluated as JavaScript. Use a typographic `’`. +The usual cause is an object literal one brace short. + +**A quote character is worse, because it does not log anything.** These +annotations are evaluated as JavaScript literals, so a `'` or a `"` inside a +single-quoted description ends the string early — and for a `"` in the middle of +a sentence the result is not an error at all. The value is silently **truncated** +at that character: + +```js +// #swagger.summary = 'A "quoted" world status' +// → "summary": "A \"" and swagger-autogen still prints Success +``` + +Nothing throws, so the generator's error capture has nothing to capture. The only +signal is `check:swagger` calling the fragment stale, with a message that blames +your routes. **If that check fires and your routes did not change, look for a +quote in an annotation first.** Backticks are safe — Markdown spans survive +verbatim. And an escaped apostrophe (`\'`) is a third case, visible only in a +rendered page: the annotation is never evaluated as JavaScript by the reader, so +Swagger UI shows the backslash. Use a typographic `’` throughout. ## Packaging and release @@ -485,3 +523,4 @@ it** — an outbound socket is not statically detectable the way an internal the next chapter is for. [api]: https://gitea.whitlocktech.com/RunicGateway/docs/src/branch/main/website/MODULE_API.md +[acceptance]: https://gitea.whitlocktech.com/RunicGateway/docs/src/branch/main/modules/kit-acceptance.md diff --git a/ci/core-ref.json b/ci/core-ref.json index a01c9cd..358c111 100644 --- a/ci/core-ref.json +++ b/ci/core-ref.json @@ -1,12 +1,14 @@ { "repo": "https://gitea.whitlocktech.com/RunicGateway/website.git", "branch": "edge", - "ref": "1b692bf624404f9e4f924c231acfbfb7e9d0861a", + "ref": "4ad8b2bb0ede2747622075dcfa4cb1fe460f91ca", "why": [ "The core this kit is written against, pinned to a commit rather than a branch.", - "This one is the MODULE_API_VERSION 1.4.0 bump, which is the version", - "template/module.json declares - slice 0 pinned its parent, before 1.4.0", - "existed, and the check below could not have passed against it.", + "This one is the MODULE_API_VERSION 1.5.0 bump, which is the version", + "template/module.json declares. It moved here from the 1.4.0 bump because", + "the kit's acceptance run found PublicLayout had no way to give a module", + "page the site's body wrapper, and core grew a `shell` prop for it - so the", + "template now uses a member that only exists at this ref and later.", "", "Moving this pin is the moment someone re-reads the chapters: CI asserts the", "version template/module.json declares still equals this core's", diff --git a/scripts/checkRenameSites.js b/scripts/checkRenameSites.js index 9533e27..d6c61fe 100644 --- a/scripts/checkRenameSites.js +++ b/scripts/checkRenameSites.js @@ -34,9 +34,23 @@ const TEMPLATE = path.join(ROOT, 'template') const CHECKLIST = path.join(TEMPLATE, 'README.md') // Anything a rename has to touch: the id (`examplegame`), the display name -// ("Example Game"), and the placeholder world ("Example World"). One pattern -// rather than three, because they are one decision. -const PLACEHOLDER = /example[ -]?(game|world)/i +// ("Example Game"), the placeholder world ("Example World"), and the two +// publishing placeholders in the Gitea release workflow (`gitea.example.com`, +// `your-org/your-module`). One pattern rather than four, because they are one +// decision — everything a reader must change before this template is theirs. +// +// **Every alternative has to be a string that cannot occur by accident**, which +// is the same rule that made the id `examplegame` rather than `example` (see the +// header). The publishing pair was added after the acceptance run found the +// release workflow carrying `# CHANGE THESE` placeholders that the checklist did +// not list and this pattern could not see — CI silent by construction +// (docs/modules/kit-acceptance.md, F3). +// +// The near-miss is worth keeping: the obvious widening is `example\.com`, and it +// is WRONG. `server/test/checkImports.test.js` uses `https://example.com/x` as a +// fixture — a URL in a string, testing that a URL in a string is not an import — +// and it is not a rename site. The host is matched in full instead. +const PLACEHOLDER = /example[ -]?(game|world)|gitea\.example\.com|your-(org|module)/i // Directories with nothing of ours in them. `dist` and `node_modules` are build // output — a chunk full of the placeholder is not a rename site, it is the diff --git a/scripts/checkRenameSites.test.js b/scripts/checkRenameSites.test.js index d4027db..aff71d5 100644 --- a/scripts/checkRenameSites.test.js +++ b/scripts/checkRenameSites.test.js @@ -61,6 +61,10 @@ test('the placeholder pattern matches every form a rename touches', () => { "worldName: 'Example World'", 'examplegame_world_status', 'example-game', + // The publishing pair, added after the acceptance run found the release + // workflow unlisted and unmatchable (kit-acceptance.md F3). + ' GITEA_HOST: gitea.example.com', + ' REPO: your-org/your-module', ]) { assert.ok(PLACEHOLDER.test(text), `should match: ${text}`) } @@ -75,6 +79,14 @@ test('the placeholder pattern does not fire on ordinary prose', () => { 'an example of what to catch', 'exampleValue', 'the game world', + // A real span from server/test/checkImports.test.js, and the reason the + // publishing host is matched in full rather than as `example.com`: it is a + // fixture URL inside a string, in a test about URLs inside strings, and it is + // not a rename site. The obvious widening would have failed the build on it. + "const url = 'https://example.com/x'", + // Prose about the reader's own module, which is not the hyphenated token. + 'copy your module directory onto the volume', + 'your org will need a release token', ]) { assert.ok(!PLACEHOLDER.test(text), `should not match: ${text}`) } diff --git a/template/.gitattributes b/template/.gitattributes new file mode 100644 index 0000000..34be3fd --- /dev/null +++ b/template/.gitattributes @@ -0,0 +1,22 @@ +# Check every text file out with LF, on every platform. +# +# This exists because of a real failure, reported by the kit's acceptance run +# (docs/modules/kit-acceptance.md, finding F1): on a default Windows clone, +# `npm run check:swagger` failed on a PRISTINE, unedited template. The check +# compares the committed swagger-fragment.json against what the generator writes; +# the generator writes LF, and core.autocrlf had handed the reader CRLF. The +# message blamed "the routes or their annotations", which is the first command the +# kit tells a reader to run telling them a false thing about their own work. +# +# The check itself now normalises line endings before comparing, so this file is +# the belt to that pair of braces: it also stops a CRLF blob ever being COMMITTED +# by a reader who copies this template, which would break the same check for +# everyone who cloned their repo afterwards. +# +# It travels with the template on purpose — a copied module directory keeps its +# own attributes, and this is one of the things the copier should not have to know. +* text=auto eol=lf + +# Nothing here is binary today. If your module ships an image or a font, mark it, +# because `text=auto` guesses and a wrong guess corrupts the file: +# *.png binary diff --git a/template/.gitea/workflows/release.yml b/template/.gitea/workflows/release.yml index 5718407..d7dc40b 100644 --- a/template/.gitea/workflows/release.yml +++ b/template/.gitea/workflows/release.yml @@ -151,12 +151,23 @@ jobs: # The server half, minus everything that never runs inside core's # process: no `test/`, no `scripts/`, no `swagger/`. + # + # An EXCLUSION list, not an include list, and that is the whole point. + # This was `for d in boot.js core.js index.js db model router`, which + # meant adding `server/utils/` — an ordinary thing to do — silently + # dropped it from every release: the bundle check below only resolves + # the five paths module.json names, so nothing failed here, and the + # module died on an operator's box as a `startup_failed` row instead + # (docs/modules/kit-acceptance.md, F4). Excluding is the safe default + # because the failure mode inverts: forget to exclude something and you + # ship a harmless extra file, rather than omitting a required one. mkdir -p "$OUT/server" - for d in boot.js core.js index.js db model router; do - cp -r "server/$d" "$OUT/server/" + for e in server/*; do + case "$(basename "$e")" in + test|scripts|swagger|package-lock.json) continue ;; + esac + cp -r "$e" "$OUT/server/" done - cp server/package.json "$OUT/server/" - [ -d server/node_modules ] && cp -r server/node_modules "$OUT/server/" || true # The client half is the BUILT chunk only. `client/src` is source an # operator has no use for and core will never read. diff --git a/template/.github/workflows/release.yml b/template/.github/workflows/release.yml index ce30456..05d2e24 100644 --- a/template/.github/workflows/release.yml +++ b/template/.github/workflows/release.yml @@ -135,12 +135,23 @@ jobs: # The server half, minus everything that never runs inside core's # process: no `test/`, no `scripts/`, no `swagger/`. + # + # An EXCLUSION list, not an include list, and that is the whole point. + # This was `for d in boot.js core.js index.js db model router`, which + # meant adding `server/utils/` — an ordinary thing to do — silently + # dropped it from every release: the bundle check below only resolves + # the five paths module.json names, so nothing failed here, and the + # module died on an operator's box as a `startup_failed` row instead + # (docs/modules/kit-acceptance.md, F4). Excluding is the safe default + # because the failure mode inverts: forget to exclude something and you + # ship a harmless extra file, rather than omitting a required one. mkdir -p "$OUT/server" - for d in boot.js core.js index.js db model router; do - cp -r "server/$d" "$OUT/server/" + for e in server/*; do + case "$(basename "$e")" in + test|scripts|swagger|package-lock.json) continue ;; + esac + cp -r "$e" "$OUT/server/" done - cp server/package.json "$OUT/server/" - [ -d server/node_modules ] && cp -r server/node_modules "$OUT/server/" || true # The client half is the BUILT chunk only. mkdir -p "$OUT/client/dist" diff --git a/template/.gitignore b/template/.gitignore new file mode 100644 index 0000000..e679066 --- /dev/null +++ b/template/.gitignore @@ -0,0 +1,36 @@ +# The template's own ignore rules, so they travel with a copy. +# +# The kit repo's root .gitignore covers these paths too, but that file stays +# behind: copy `template/` out, `git init`, and you inherit nothing — `node_modules/` +# included. Reported by the acceptance run (docs/modules/kit-acceptance.md, F6). + +# dependencies +node_modules/ + +# ── The built client chunk ──────────────────────────────────────────────── +# +# Ignored HERE and shipped in the RELEASE, which is not a contradiction: a module +# is installed prebuilt (an operator never builds anything), but the artifact is +# built by CI from the source next to it, and a chunk committed by hand goes stale +# beside fresh source without anything saying so. +# +# Your release workflow builds it before packing the bundle. If you would rather +# commit it, delete this line and accept that you now have to remember. +client/dist/ + +# env / secrets — a module's env vars are the operator's, never the repo's +.env +*.env +!.env.example + +# release staging, produced by .gitea/workflows/release.yml +/dist/ +*.tar.gz + +# logs / os / editor +*.log +npm-debug.log* +.DS_Store +Thumbs.db +.vscode/ +.idea/ diff --git a/template/README.md b/template/README.md index 04b17c0..ed43485 100644 --- a/template/README.md +++ b/template/README.md @@ -35,7 +35,7 @@ server/ client/ vite.config.js the library build: anchored aliases, external: [] src/entry.jsx registers routes and nav at evaluation time - src/core.js what core hands you: the seven-member UI kit + src/core.js what core hands you: the UI kit (eight exports) src/shim/ the four shared dependencies, re-exported from core src/routes/public/ the page scripts/checkExternals.js asks the BUILT chunk whether a bare import survived @@ -103,6 +103,7 @@ backticking table names**. | File | What to change | | --- | --- | | `module.json` | `id`, `name`, `version`, the `mounts` prefix, `capabilities` | +| `.gitea/workflows/release.yml` | `GITEA_HOST` and `REPO`, under the `# CHANGE THESE` banner — the only two, and they are wrong until you do. (The `.github/` flavour needs nothing: GitHub supplies `GITHUB_REPOSITORY` and friends.) | | `server/package.json` | package `name` and `description` | | `server/core.js` | the message every accessor throws | | `server/boot.js` | the placeholder world name | diff --git a/template/client/src/core.js b/template/client/src/core.js index 74bd4a4..3f7b90b 100644 --- a/template/client/src/core.js +++ b/template/client/src/core.js @@ -45,14 +45,14 @@ if (createElement !== rg.react.createElement || createRoot !== rg.reactDom.creat ) } -// The curated kit (§3.4). Seven members, and it is CLOSED: layout, headings, the +// The curated kit (§3.4). Eight exports, and it is CLOSED: layout, headings, the // three data-page states, the fetch hook, and read-only access to the session and // the site's settings. Anything else your pages need — tables, tabs, an editor — // you bundle yourself, in a `components/` directory of your own. // // Closed is a real constraint and it is the price of the boundary being worth -// anything: adding a member is a minor `MODULE_API_VERSION` bump, and changing a -// kit component's props is a major one. Use them, though. A module page that +// anything: adding a member is a minor `MODULE_API_VERSION` bump, and changing an +// existing prop on a kit component is a major one. Use them, though. A module page that // ships its own layout is a page that stops looking like the site it is installed // in, and drifts further every time core changes. export const { diff --git a/template/client/src/routes/public/WorldStatus.jsx b/template/client/src/routes/public/WorldStatus.jsx index 3342784..a4b1a67 100644 --- a/template/client/src/routes/public/WorldStatus.jsx +++ b/template/client/src/routes/public/WorldStatus.jsx @@ -3,13 +3,24 @@ // An ordinary React component. Nothing about being inside a module changes how // you write one — the only differences are where React comes from (core, via the // aliases in vite.config.js, so the import below looks completely normal and is -// not) and where the chrome comes from (`../../core.js`, the seven-member kit). +// not) and where the chrome comes from (`../../core.js`, the shared UI kit). // // **Render `PublicLayout` yourself.** Core wraps your public routes in its // maintenance gate and nothing else, so a page that omits the layout renders // bare — no header, no footer, no site chrome — which looks like a bug and is // the contract (§3.3). Admin and player routes are the other way round: core // wraps those in their layouts for you. +// +// **And pass a `shell`.** The layout is the chrome; `shell` is the body — the +// centred column, the vertical padding, and the thing that holds the footer at +// the bottom of the viewport. Without it your content starts hard against the +// left edge of the window and the footer rides up underneath it, which reads as +// a CSS bug in your module and is not one. Widths are 'narrow', 'mid' and +// 'wide'; name a width, never a class, because the classes belong to core's +// stylesheet and it is free to rename them (§3.4, MODULE_API_VERSION 1.5.0). +// +// This is here because the kit's acceptance run got it wrong by following the +// kit: a module built to the letter of chapter 2 rendered outside the site. import { ErrorState, Loading, PageHeader, PublicLayout, useAsync } from '../../core.js' import api from '../../api.js' @@ -34,7 +45,7 @@ export default function WorldStatus() { const { data, loading, error } = useAsync(() => api.world.status(), []) return ( - + s.replace(/\r\n/g, '\n') + + if (lf(fs.readFileSync(FRAGMENT, 'utf8')) !== lf(json)) { process.stderr.write( '\nswagger-fragment.json is STALE — the routes or their annotations changed and it was not\n' + 'regenerated. Run `npm run swagger` and commit the result. Core merges this file verbatim,\n' + diff --git a/template/server/test/noGameConnection.test.js b/template/server/test/noGameConnection.test.js new file mode 100644 index 0000000..14499cd --- /dev/null +++ b/template/server/test/noGameConnection.test.js @@ -0,0 +1,124 @@ +// ── §2.7's last rule, given the CI it does not have ─────────────────────── +// +// `book/02-website-module.md` is explicit that "the website process never opens a +// connection to a game server" is the **one boundary rule with no CI behind it**: +// an outbound socket is not statically detectable the way an internal `require` +// is, so in general the rule is held up by review and by understanding it. +// +// True of the general case, and not a reason to check nothing. A module can state +// a narrower, completely decidable property about **itself**, and this one says: +// the shipped server half references no networking primitive at all. Everything +// it knows arrives from its own tables, which its sidecar writes. +// +// Adopted from the kit's acceptance run (`docs/modules/kit-acceptance.md`), where +// a reader building a Rust module wrote it unprompted after reading that the rule +// had no CI — and observed that for Rust in particular, which ships RCON over +// WebSocket, `new WebSocket(rconUrl)` in `boot.js` is about ten lines away. +// +// ── WHEN YOU ADD A SIDECAR CLIENT, NARROW THIS. DO NOT DELETE IT. ───────── +// +// Talking to *your sidecar* over HTTP is the expected shape and is not what §2.7 +// forbids — the rule is about the **game server**. So the moment your module +// grows, say, `server/sidecarClient.js`, this test starts failing correctly and +// the fix is to allow that one file: +// +// const MAY_OPEN_SOCKETS = new Set(['sidecarClient.js']) +// +// and keep the rest of the tree under the ban. What you get for that is a test +// that names the *one* file allowed to reach the network — which is exactly the +// file a reviewer should be reading closely, and exactly the place a game-server +// URL would appear if the rule were ever broken. +// +// Scope: SHIPPED code only. `test/` and `scripts/` never run inside core's process. + +const test = require('node:test') +const assert = require('node:assert') +const fs = require('node:fs') +const path = require('node:path') + +const SERVER_ROOT = path.resolve(__dirname, '..') +const NOT_SHIPPED = new Set(['test', 'scripts', 'node_modules', 'swagger']) + +/** Every shipped `.js` file under `server/`. */ +function shippedFiles(dir = SERVER_ROOT, out = []) { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + if (entry.isDirectory()) { + if (dir === SERVER_ROOT && NOT_SHIPPED.has(entry.name)) continue + if (entry.name === 'node_modules') continue + shippedFiles(path.join(dir, entry.name), out) + } else if (entry.isFile() && entry.name.endsWith('.js')) { + out.push(path.join(dir, entry.name)) + } + } + return out +} + +/** + * Blank comments, so prose ABOUT the rule does not trip the rule. + * + * This file is itself the proof that it is needed: the paragraphs above say + * "WebSocket" several times. `scripts/checkImports.js` documents hitting exactly + * this on its own documentation, and it is the third time in this project's + * history that a boundary check has failed on the text explaining it. + * + * Blanked rather than deleted, so line numbers in a failure still point at the + * right line. + */ +function stripComments(src) { + return src + .replace(/\/\*[\s\S]*?\*\//g, (m) => m.replace(/[^\n]/g, ' ')) + .replace(/^[ \t]*\/\/.*$/gm, '') +} + +// Each is a way a Node process opens a socket. Matched as identifiers, so a +// column named `websocket_url` inside a SQL string would not fire. +const NETWORKING = [ + /\brequire\(\s*['"](?:node:)?(?:net|tls|dgram|http|https|http2)['"]\s*\)/, + /\bfrom\s+['"](?:node:)?(?:net|tls|dgram|http|https|http2)['"]/, + /\brequire\(\s*['"](?:ws|socket\.io-client|undici|axios|node-fetch|got)['"]\s*\)/, + /\bnew\s+WebSocket\b/, + /\bfetch\s*\(/, + /\bXMLHttpRequest\b/, + /\bEventSource\b/, +] + +test('no shipped file references a networking primitive (§2.7)', () => { + const offenders = [] + for (const file of shippedFiles()) { + const code = stripComments(fs.readFileSync(file, 'utf8')) + for (const pattern of NETWORKING) { + if (pattern.test(code)) { + offenders.push(`${path.relative(SERVER_ROOT, file)} matches ${pattern}`) + } + } + } + assert.deepStrictEqual( + offenders, + [], + 'the website process must never open a connection to a game server. If this is ' + + 'your sidecar client, allow that one file rather than removing the check — see ' + + `the header of this file.\n ${offenders.join('\n ')}`, + ) +}) + +test('the check can actually fail — it is pointed at a real violation', () => { + // A check that has never been shown to fail is a check nobody knows the state + // of. This is the game-server dial the rule exists to stop. + const violation = "const socket = new WebSocket('ws://10.0.0.5:28016/' + rconPassword)" + assert.ok( + NETWORKING.some((p) => p.test(stripComments(violation))), + 'the guard would not have caught a direct game-server dial', + ) +}) + +test('prose describing the rule does not trip it', () => { + const prose = [ + '// A game shipping RCON over WebSocket means a module COULD write', + "// const s = new WebSocket(url); require('net')", + '// in about ten lines. It must not.', + 'const x = 1', + ].join('\n') + for (const pattern of NETWORKING) { + assert.ok(!pattern.test(stripComments(prose)), `${pattern} fired on a comment`) + } +})