feat(modules): the declarative Docker path (phase 4, slice 3)

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>

server/test/adminModules.test.js

@@ -29,6 +29,7 @@ const settings = require('../src/model/settings/settings.model')
 const loader = require('../src/modules/loader')
 const lifecycle = require('../src/modules/lifecycle')
 const install = require('../src/modules/install')
+const declared = require('../src/modules/declared')
 const schema = require('../src/modules/schema')
 const db = require('../src/utils/db')
 
@@ -67,6 +68,7 @@ const originals = {
     removeDir: install.removeDir,
   },
   schema: { runPurge: schema.runPurge },
+  declared: { state: declared.state },
 }
 
 let logged
@@ -79,6 +81,7 @@ beforeEach(() => {
   Object.assign(lifecycle, originals.lifecycle)
   Object.assign(install, originals.install)
   Object.assign(schema, originals.schema)
+  Object.assign(declared, originals.declared)
 
   logged = []
   activity.log = async (entry) => { logged.push(entry) }
@@ -133,6 +136,55 @@ test('list includes a module on the volume that has no row yet', async () => {
   assert.equal(res.body.modules[0].liveState, 'started')
 })
 
+test('list carries what MODULES declares, including a module it could not install', async () => {
+  // Slice 3's fourth source. A declared module that failed to resolve has no
+  // row, no directory and nothing mounted — so it is invisible to the other
+  // three, and the reason it is missing is the one thing the operator needs.
+  modules.list = async () => [{
+    id: 'uo', name: 'UO', version: '0.2.0', state: 'enabled',
+    failureStage: null, failureReason: null, source: null, sha256: null,
+    installedAt: null, startedAt: null,
+  }]
+  loader.list = () => [{ id: 'uo', name: 'UO', version: '0.2.0', state: 'started', stage: null, reason: null, capabilities: [] }]
+  install.isInstalled = (id) => id === 'uo'
+  install.purgeFile = () => null
+  declared.state = () => [
+    { id: 'uo', version: '0.3.0', url: 'https://x/uo.json', action: 'failed', message: 'the host is down' },
+    { id: 'market', version: '1.0.0', url: 'https://x/market.json', action: 'failed', message: 'not found' },
+  ]
+
+  const res = mockRes()
+  await ctrl.list(req(), res)
+
+  const byId = Object.fromEntries(res.body.modules.map((m) => [m.id, m]))
+  // Running fine at 0.2.0 while the declared upgrade to 0.3.0 is failing: both
+  // facts survive, because collapsing them would have to discard one.
+  assert.equal(byId.uo.liveState, 'started')
+  assert.equal(byId.uo.declared, true)
+  assert.equal(byId.uo.declaredVersion, '0.3.0')
+  assert.equal(byId.uo.declaredError, 'the host is down')
+  // Declared and nowhere: listed anyway, with no version invented for it.
+  assert.equal(byId.market.declared, true)
+  assert.equal(byId.market.version, null)
+  assert.equal(byId.market.state, null)
+  assert.equal(byId.market.declaredError, 'not found')
+})
+
+test('a successfully resolved module is marked declared, with no error', async () => {
+  modules.list = async () => []
+  loader.list = () => [{ id: 'uo', name: 'UO', version: '0.3.0', state: 'started', stage: null, reason: null, capabilities: [] }]
+  install.isInstalled = () => true
+  install.purgeFile = () => null
+  declared.state = () => [{ id: 'uo', version: '0.3.0', url: 'https://x/uo.json', action: 'noop', message: null }]
+
+  const res = mockRes()
+  await ctrl.list(req(), res)
+
+  assert.equal(res.body.modules.length, 1, 'declared and present is ONE module, not two')
+  assert.equal(res.body.modules[0].declared, true)
+  assert.equal(res.body.modules[0].declaredError, null)
+})
+
 test('list survives a process where the loader never scanned', async () => {
   loader.isLoaded = () => false
   loader.list = () => { throw new Error('modules.list() before modules.load()') }

server/test/bootOrder.test.js

@@ -0,0 +1,82 @@
+// server.js's boot ORDER, which is a contract and not a style choice.
+//
+// Phase 4, slice 3 of MODULE_SYSTEM.md §2.7.2. Five steps have to happen in one
+// order, and each arrow is a dependency that is invisible at the call site:
+//
+//   core schema  →  the declared set (§2.7.2 decision 4) needs the settings row
+//                   its seed writes, to know which hosts it may install from
+//   declared set →  requiring app.js SCANS the volume (§1.12), so anything put
+//                   there afterwards is not in this process
+//   require app  →  module schema fragments (§2.6) can only be replayed once the
+//                   loader knows which modules there are
+//   fragments    →  onBoot runs against tables that exist
+//
+// This is a source-structure test, and it is worth being plain about what that
+// does and does not prove: it cannot tell you the server boots, only that nobody
+// has quietly moved one of these five lines past another. It exists because the
+// defect it guards against has already happened once and was invisible to every
+// other kind of test here. Deferring the app require — which slice 3 had to do —
+// moved core's schema ahead of the scan, and `ensureSchema` then skipped the
+// module fragments entirely. On this machine's dev database the tables already
+// existed, so the module started; on a FRESH database it would have started
+// against no tables at all. Every suite in this directory stubs either the
+// loader or the pool, so none of them could see it. The browser smoke did, from
+// one log line.
+process.env.DB_HOST = '127.0.0.1'
+process.env.DB_PORT = '59999'
+
+const fs = require('fs')
+const path = require('path')
+
+const { test } = require('node:test')
+const assert = require('node:assert/strict')
+
+const SERVER = fs.readFileSync(path.join(__dirname, '..', 'src', 'server.js'), 'utf8')
+
+/** Where a marker appears, asserted to appear exactly once. */
+function at(marker) {
+  const first = SERVER.indexOf(marker)
+  assert.notEqual(first, -1, `server.js no longer contains ${JSON.stringify(marker)}`)
+  assert.equal(
+    SERVER.indexOf(marker, first + 1),
+    -1,
+    `${JSON.stringify(marker)} appears more than once in server.js — this test cannot tell which is the boot step`,
+  )
+  return first
+}
+
+test('boot runs core schema, then the declared set, then the scan, then fragments, then onBoot', () => {
+  const steps = [
+    ['core schema', at('await ensureSchema({ replayModules: false })')],
+    ['declared module set', at('await declaredModules.resolve(')],
+    ['the volume scan', at("require('./app')")],
+    ['module schema fragments', at('replayFragments()')],
+    ['module onBoot', at('await moduleLifecycle.boot()')],
+  ]
+
+  for (let i = 1; i < steps.length; i += 1) {
+    assert.ok(
+      steps[i][1] > steps[i - 1][1],
+      `${steps[i][0]} must come after ${steps[i - 1][0]} in server.js`,
+    )
+  }
+})
+
+test('app.js is required inside start(), never at the top of the file', () => {
+  // The whole mechanism depends on this. A top-level require runs when server.js
+  // is loaded — before a single line of start() — and the scan would then happen
+  // before the declared set had a chance to put anything on the volume, silently
+  // and with no error anywhere.
+  assert.ok(
+    at("require('./app')") > at('async function start()'),
+    'requiring ./app at the top of server.js scans the volume before the declared set is resolved',
+  )
+})
+
+test('ensureSchema is asked NOT to replay fragments, and something else does', () => {
+  // Both halves matter. Passing the flag without replaying elsewhere is the
+  // original defect with an explicit spelling; replaying without the flag runs
+  // the fragments twice, the second time being the one that matters.
+  assert.match(SERVER, /ensureSchema\(\{ replayModules: false \}\)/)
+  assert.match(SERVER, /modules\/schema'\)\.replayFragments\(\)/)
+})

server/test/moduleDeclared.test.js

@@ -0,0 +1,391 @@
+// modules/declared.js — resolving the module set an environment declares.
+//
+// Phase 4, slice 3 of MODULE_SYSTEM.md §2.7.2, decision 4. Two claims are worth
+// more than the rest and both are about what does NOT happen:
+//
+//   - a module already unpacked at the declared version does not touch the
+//     network, because that is what makes a restart with the internet down come
+//     up unchanged;
+//   - a module that cannot be resolved does not fail the boot, and does not stop
+//     the next declaration from resolving.
+//
+// The last test in the file runs the whole path through the real install.js
+// against a fake transport — a real gzipped tar, really hashed, really unpacked
+// — because everything above it stubs `installImpl` and would keep passing if
+// the two files stopped agreeing about what an install returns.
+process.env.DB_HOST = '127.0.0.1'
+process.env.DB_PORT = '59999'
+
+const crypto = require('crypto')
+const fs = require('fs')
+const os = require('os')
+const path = require('path')
+const zlib = require('zlib')
+
+const { test, beforeEach, after } = require('node:test')
+const assert = require('node:assert/strict')
+
+const db = require('../src/utils/db')
+
+after(() => db.close())
+
+const HOSTS = ['releases.example.com']
+const URL_030 = 'https://releases.example.com/mod/uo-0.3.0.json'
+
+let tmpRoot
+let declared
+
+/**
+ * A fresh declared.js bound to a fresh modules directory.
+ *
+ * loader.js resolves MODULES_DIR once at require time and install.js reads
+ * `loader.dir()`, so all three have to come back together — the same dance
+ * moduleInstall.test.js and moduleLifecycle.test.js do.
+ */
+function fresh(dir) {
+  process.env.MODULES_DIR = dir
+  for (const m of ['loader', 'install', 'declared']) {
+    delete require.cache[require.resolve(`../src/modules/${m}`)]
+  }
+  // eslint-disable-next-line global-require
+  return require('../src/modules/declared')
+}
+
+/** Put a module directory on the volume, as an unpack would leave it. */
+function place(id, version) {
+  const dir = path.join(tmpRoot, id)
+  fs.mkdirSync(dir, { recursive: true })
+  fs.writeFileSync(
+    path.join(dir, 'module.json'),
+    JSON.stringify({ id, name: 'Ultima Online', version, coreApi: '^1.0.0' }),
+  )
+  return dir
+}
+
+/**
+ * A stub install service.
+ *
+ * Records every call, so "did this reach the network at all" is a question the
+ * tests can ask directly rather than inferring from an outcome.
+ */
+function fakeInstall({ version = '0.3.0', throws = null } = {}) {
+  const calls = []
+  return {
+    calls,
+    InstallError: Error,
+    async install(args) {
+      calls.push(args)
+      if (throws) throw new Error(throws)
+      return {
+        id: 'uo',
+        name: 'Ultima Online',
+        version,
+        sha256: 'a'.repeat(64),
+        source: args.url,
+        bytes: 1024,
+        replaced: false,
+      }
+    },
+  }
+}
+
+/** A modules.model stand-in that remembers what provenance it was given. */
+function fakeModel() {
+  const recorded = []
+  return { recorded, async recordInstalled(row) { recorded.push(row); return row } }
+}
+
+beforeEach(() => {
+  tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'rg-declared-'))
+  declared = fresh(tmpRoot)
+})
+
+// ── Parsing ────────────────────────────────────────────────────────────────
+
+test('parses id@version=url entries separated by whitespace or commas', () => {
+  const { entries, errors } = declared.parse(
+    ` uo@0.3.0=https://a.example/uo.json,\n  market@1.2.3=https://a.example/market.json `,
+  )
+  assert.deepEqual(errors, [])
+  assert.deepEqual(entries, [
+    { id: 'uo', version: '0.3.0', url: 'https://a.example/uo.json' },
+    { id: 'market', version: '1.2.3', url: 'https://a.example/market.json' },
+  ])
+})
+
+test('an empty or unset declaration parses to nothing, without complaint', () => {
+  for (const value of [undefined, '', '   ']) {
+    const { entries, errors } = declared.parse(value)
+    assert.deepEqual(entries, [])
+    assert.deepEqual(errors, [])
+  }
+})
+
+test('a malformed entry is refused on its own, leaving the others', () => {
+  // A bare URL, a missing version, and an id that is not one — each of which an
+  // operator can plausibly type, and none of which should cost the module that
+  // was written correctly.
+  const { entries, errors } = declared.parse(
+    'https://a.example/uo.json uo@=https://a.example/uo.json Uo@1=https://a.example/uo.json'
+      + ' good@1.0.0=https://a.example/good.json',
+  )
+  assert.equal(entries.length, 1)
+  assert.equal(entries[0].id, 'good')
+  assert.equal(errors.length, 3)
+})
+
+test('a duplicate id keeps the first and says so', () => {
+  const { entries, errors } = declared.parse(
+    'uo@1.0.0=https://a.example/one.json uo@2.0.0=https://a.example/two.json',
+  )
+  assert.equal(entries.length, 1)
+  assert.equal(entries[0].version, '1.0.0')
+  assert.match(errors[0], /declared more than once/)
+})
+
+// ── Resolution ─────────────────────────────────────────────────────────────
+
+test('a module already at the declared version is a no-op that never fetches', async () => {
+  place('uo', '0.3.0')
+  const installer = fakeInstall()
+  const model = fakeModel()
+
+  const results = await declared.resolve({
+    value: `uo@0.3.0=${URL_030}`,
+    hosts: HOSTS,
+    model,
+    installImpl: installer,
+  })
+
+  assert.deepEqual(results.map((r) => r.action), ['noop'])
+  // The offline guarantee, stated as an assertion: nothing was fetched and
+  // nothing was written. A restart with no route to the internet is this case.
+  assert.equal(installer.calls.length, 0)
+  assert.equal(model.recorded.length, 0)
+})
+
+test('a missing module is installed, with its provenance recorded', async () => {
+  const installer = fakeInstall()
+  const model = fakeModel()
+
+  const results = await declared.resolve({
+    value: `uo@0.3.0=${URL_030}`,
+    hosts: HOSTS,
+    model,
+    installImpl: installer,
+  })
+
+  assert.deepEqual(results.map((r) => r.action), ['installed'])
+  assert.equal(installer.calls[0].url, URL_030)
+  assert.deepEqual(installer.calls[0].hosts, HOSTS)
+  // The declaration is handed down so install.js can refuse a URL that turns out
+  // to be another module or another version BEFORE it downloads it.
+  assert.deepEqual(installer.calls[0].expect, { id: 'uo', version: '0.3.0' })
+  // Written exactly as the admin route writes it — the reason this runs in the
+  // server process rather than in a script that cannot reach the database.
+  assert.deepEqual(model.recorded, [
+    { id: 'uo', name: 'Ultima Online', version: '0.3.0', source: URL_030, sha256: 'a'.repeat(64) },
+  ])
+})
+
+test('a module at a different version is re-resolved', async () => {
+  place('uo', '0.2.0')
+  const installer = fakeInstall()
+
+  const results = await declared.resolve({
+    value: `uo@0.3.0=${URL_030}`,
+    hosts: HOSTS,
+    model: fakeModel(),
+    installImpl: installer,
+  })
+
+  assert.deepEqual(results.map((r) => r.action), ['installed'])
+  assert.equal(installer.calls.length, 1)
+})
+
+test('a directory with no readable module.json counts as absent', async () => {
+  fs.mkdirSync(path.join(tmpRoot, 'uo'), { recursive: true })
+  fs.writeFileSync(path.join(tmpRoot, 'uo', 'module.json'), '{ this is not json')
+  const installer = fakeInstall()
+
+  await declared.resolve({
+    value: `uo@0.3.0=${URL_030}`,
+    hosts: HOSTS,
+    model: fakeModel(),
+    installImpl: installer,
+  })
+
+  // Whatever is in that directory, it is not the declared module — so the
+  // declared module is fetched rather than assumed to be there.
+  assert.equal(installer.calls.length, 1)
+})
+
+test('a failure is carried, not thrown, and does not stop the next module', async () => {
+  const model = fakeModel()
+  const installer = {
+    calls: [],
+    async install(args) {
+      installer.calls.push(args)
+      if (args.expect.id === 'uo') throw new Error('could not reach releases.example.com')
+      return {
+        id: args.expect.id,
+        name: 'Market',
+        version: args.expect.version,
+        sha256: 'b'.repeat(64),
+        source: args.url,
+        bytes: 10,
+        replaced: false,
+      }
+    },
+  }
+
+  const results = await declared.resolve({
+    value: `uo@0.3.0=${URL_030} market@1.0.0=https://releases.example.com/market.json`,
+    hosts: HOSTS,
+    model,
+    installImpl: installer,
+  })
+
+  assert.deepEqual(results.map((r) => r.action), ['failed', 'installed'])
+  assert.match(results[0].message, /could not reach/)
+  // The second module still installed: one unreachable release host costs that
+  // module, not the deployment.
+  assert.equal(model.recorded.length, 1)
+  assert.equal(model.recorded[0].id, 'market')
+})
+
+test('a failed resolution leaves whatever was already on the volume', async () => {
+  place('uo', '0.2.0')
+  const installer = fakeInstall({ throws: 'the host is down' })
+
+  const results = await declared.resolve({
+    value: `uo@0.3.0=${URL_030}`,
+    hosts: HOSTS,
+    model: fakeModel(),
+    installImpl: installer,
+  })
+
+  assert.equal(results[0].action, 'failed')
+  // The site comes up serving the version it already had rather than not at all.
+  assert.equal(declared.installedVersion('uo'), '0.2.0')
+})
+
+test('resolution without a model records nothing and still installs', async () => {
+  // `npm run seed` and the test harness both reach code paths with no model to
+  // hand; the volume half must not depend on the database half.
+  const installer = fakeInstall()
+  const results = await declared.resolve({ value: `uo@0.3.0=${URL_030}`, hosts: HOSTS, installImpl: installer })
+  assert.deepEqual(results.map((r) => r.action), ['installed'])
+})
+
+test('state() reports the last resolution and is replaced by the next', async () => {
+  const installer = fakeInstall()
+  await declared.resolve({ value: `uo@0.3.0=${URL_030}`, hosts: HOSTS, installImpl: installer })
+  assert.equal(declared.state().length, 1)
+  assert.equal(declared.state()[0].id, 'uo')
+
+  await declared.resolve({ value: '', hosts: HOSTS, installImpl: installer })
+  assert.deepEqual(declared.state(), [])
+})
+
+// ── The whole path, once, for real ─────────────────────────────────────────
+
+const BLOCK = 512
+
+function octal(value, width) {
+  return Number(value).toString(8).padStart(width - 1, '0') + '\0'
+}
+
+function member({ name, type = '0', body = '' }) {
+  const header = Buffer.alloc(BLOCK, 0)
+  const data = Buffer.from(body, 'utf8')
+  header.write(name, 0, 100, 'utf8')
+  header.write(octal(0o644, 8), 100, 8, 'ascii')
+  header.write(octal(0, 8), 108, 8, 'ascii')
+  header.write(octal(0, 8), 116, 8, 'ascii')
+  header.write(octal(data.length, 12), 124, 12, 'ascii')
+  header.write(octal(0, 12), 136, 12, 'ascii')
+  header.write('        ', 148, 8, 'ascii')
+  header.write(type, 156, 1, 'ascii')
+  header.write('ustar\0', 257, 6, 'ascii')
+  header.write('00', 263, 2, 'ascii')
+  let sum = 0
+  for (const byte of header) sum += byte
+  header.write(`${sum.toString(8).padStart(6, '0')}\0 `, 148, 8, 'ascii')
+  const padding = Buffer.alloc((BLOCK - (data.length % BLOCK)) % BLOCK, 0)
+  return Buffer.concat([header, data, padding])
+}
+
+/** A bundle tarball, named the way module-uo's release workflow names one. */
+function bundle(version) {
+  const root = `module-uo-${version}`
+  return zlib.gzipSync(Buffer.concat([
+    member({ name: `${root}/`, type: '5' }),
+    member({
+      name: `${root}/module.json`,
+      body: JSON.stringify({ id: 'uo', name: 'Ultima Online', version, coreApi: '^1.0.0', server: 'server/index.js' }),
+    }),
+    member({ name: `${root}/server/`, type: '5' }),
+    member({ name: `${root}/server/index.js`, body: 'module.exports = () => {}\n' }),
+    Buffer.alloc(BLOCK * 2, 0),
+  ]))
+}
+
+test('end to end: a declaration installs a real bundle through the real install path', async (t) => {
+  const tarball = bundle('0.3.0')
+  const artifactUrl = 'https://releases.example.com/mod/uo-0.3.0.tar.gz'
+  const manifest = JSON.stringify({
+    schema: 1,
+    id: 'uo',
+    name: 'Ultima Online',
+    version: '0.3.0',
+    coreApi: '^1.0.0',
+    artifact: 'uo-0.3.0.tar.gz',
+    url: artifactUrl,
+    sha256: crypto.createHash('sha256').update(tarball).digest('hex'),
+    size: tarball.length,
+  })
+  const routes = { [URL_030]: manifest, [artifactUrl]: tarball }
+  const fetched = []
+  const fetchImpl = async (url) => {
+    fetched.push(String(url))
+    const body = routes[String(url)]
+    return body === undefined ? new Response('nope', { status: 404 }) : new Response(body, { status: 200 })
+  }
+
+  // The real install.js, with only the transport replaced — same seam the
+  // install tests use, and the same reason: what is under test is the decisions,
+  // not Node's TLS.
+  // eslint-disable-next-line global-require
+  const install = require('../src/modules/install')
+  const installImpl = { install: (args) => install.install({ ...args, fetchImpl }) }
+  const model = fakeModel()
+
+  const first = await declared.resolve({
+    value: `uo@0.3.0=${URL_030}`,
+    hosts: HOSTS,
+    model,
+    installImpl,
+  })
+  assert.deepEqual(first.map((r) => r.action), ['installed'])
+  // Unpacked, with the release's top-level directory stripped, under the id.
+  assert.equal(
+    JSON.parse(fs.readFileSync(path.join(tmpRoot, 'uo', 'module.json'), 'utf8')).version,
+    '0.3.0',
+  )
+  assert.equal(model.recorded[0].sha256, JSON.parse(manifest).sha256)
+
+  // And again, which is what every restart after the first one is.
+  const before = fetched.length
+  const second = await declared.resolve({ value: `uo@0.3.0=${URL_030}`, hosts: HOSTS, model, installImpl })
+  assert.deepEqual(second.map((r) => r.action), ['noop'])
+  assert.equal(fetched.length, before, 'the second resolution must not fetch anything')
+
+  // A declaration whose URL resolves to a different version is refused BEFORE
+  // the artifact is downloaded — the module on the volume is left alone.
+  const wrong = await declared.resolve({ value: `uo@0.4.0=${URL_030}`, hosts: HOSTS, model, installImpl })
+  assert.equal(wrong[0].action, 'failed')
+  assert.match(wrong[0].message, /v0\.4\.0 was asked for/)
+  assert.equal(declared.installedVersion('uo'), '0.3.0')
+  t.diagnostic(`fetched ${fetched.length} URL(s) across three resolutions`)
+})