fix(release): ship server/commands, and check that the bundle is complete
All checks were successful
PR Checks / client-build (pull_request) Successful in 16s
PR Checks / frozen-manifest (pull_request) Successful in 40s
PR Checks / server-tests (pull_request) Successful in 8m38s

v1.0.0 installed and then died on every boot:

  module "uo" failed to load — {"stage":"register","reason":"Cannot find
  module './commands/guild.command'"}

`server/commands/` arrived with the Teams cutover (2d1d91e, `/guild`). The
release assembles the tarball from an include list, that list was hardcoded in
release.yml, and it was never told about the new directory — so the bundle
shipped without it and the module was dead on the operator's box.

Nothing caught it, and that is the more interesting half. Every PR check runs
against the whole repo — `frozen-manifest` even installs the module into core by
tarring the entire tree — but a release is a SUBSET of the repo, and the subset
exists nowhere except the release. The pre-publish check in release.yml only
stats the paths `module.json` declares, and a file reached by a require inside
`register()` is named in none of them, so it passed on a bundle that could not
load.

The include list stays an include list — release.yml's header makes that case
and it still holds. What changes is that it is declared ONCE, in ci/bundle.json,
with two readers instead of one:

  • release.yml assembles from it (via jq) rather than from its own copy.
  • server/scripts/checkBundle.js asks, in PR checks, whether it still covers
    everything `server/index.js` reaches — following requires transitively and
    through function bodies, which is where index.js deliberately puts them.

And the release gains a real loadability check: `checkBundle.js --bundle` walks
the ASSEMBLED tree and asserts every relative require resolves inside it. Asked
of the artifact rather than the source, so it also catches a half-failed copy or
a list naming a path that has moved.

Requiring the entry point would not have worked as a check: index.js requires
inside `register()` because require order is load-bearing (`core.init(ctx)` must
run before anything under `router/`), so requiring it evaluates one line and
reports success on a bundle missing every router it has.

Both modes were verified against the real defect — each fails with `commands`
removed and passes with it present.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WnDSWzpUjw8t8C2hghysNz
This commit is contained in:
2026-08-19 13:25:57 -05:00
parent 16cfbe194d
commit 3c179e3338
6 changed files with 540 additions and 3 deletions

View File

@@ -0,0 +1,208 @@
// The bundle check, checked.
//
// `scripts/checkBundle.js` exists because v1.0.0 shipped without
// `server/commands/` and died at the register stage on the operator's box. A
// check written in response to one bug is worth exactly as much as its coverage
// of that bug, so the first two tests below are that bug, in both modes: a list
// that has stopped covering what the entry point reaches, and a tarball with the
// file missing from it.
//
// **Every fixture is a template literal, and that is load-bearing** — the same
// reason checkImports.test.js gives. `scripts/checkImports.js` scans this
// directory too, so an ordinary quoted string holding a relative require would
// make this file fail that check. Templates are blanked by the stripper.
const test = require('node:test')
const assert = require('node:assert')
const fs = require('node:fs')
const os = require('node:os')
const path = require('node:path')
const {
reachable,
resolveFile,
checkDeclaration,
checkBundle,
declaredServerPaths
} = require('../scripts/checkBundle')
/**
* Write a throwaway module tree: `files` under server/, `bundle` as its
* ci/bundle.json. Returns the module root.
*/
function fixture(files, bundle = { server: ['index.js'] }) {
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'module-uo-bundle-'))
for (const [name, source] of Object.entries(files)) {
const file = path.join(root, 'server', name)
fs.mkdirSync(path.dirname(file), { recursive: true })
fs.writeFileSync(file, source)
}
fs.mkdirSync(path.join(root, 'ci'), { recursive: true })
fs.writeFileSync(path.join(root, 'ci', 'bundle.json'), JSON.stringify(bundle))
return root
}
const cleanup = (root) => fs.rmSync(root, { recursive: true, force: true })
// ── The regression this script was written for ─────────────────────────────
test('--check catches a directory the include list has stopped covering', () => {
const root = fixture(
{
'index.js': `const g = require('./commands/guild.command')`,
'commands/guild.command.js': `module.exports = {}`
},
{ server: ['index.js'] } // `commands` missing — exactly v1.0.0
)
try {
const { uncovered } = checkDeclaration(root)
assert.strictEqual(uncovered.size, 1)
assert.ok(uncovered.has('server/commands'))
} finally {
cleanup(root)
}
})
test('--bundle catches the file missing from an assembled tarball', () => {
const root = fixture({ 'index.js': `require('./commands/guild.command')` })
try {
const { missing } = checkBundle(root)
assert.strictEqual(missing.length, 1)
assert.strictEqual(missing[0].specifier, './commands/guild.command')
} finally {
cleanup(root)
}
})
// ── It has to reach requires that are not at the top level ─────────────────
test('follows requires written inside a function', () => {
// index.js requires inside `register()` because require order is load-bearing.
// A check that only saw file-scope requires would have missed the real bug.
const root = fixture(
{
'index.js': `module.exports = function register(ctx) { const r = require('./router/a') }`,
'router/a.js': `module.exports = {}`
},
{ server: ['index.js', 'router'] }
)
try {
assert.strictEqual(checkDeclaration(root).uncovered.size, 0)
assert.strictEqual(checkBundle(root).missing.length, 0)
} finally {
cleanup(root)
}
})
test('follows requires transitively, not just one hop', () => {
const root = fixture(
{
'index.js': `require('./a')`,
'a.js': `require('./b')`,
'b.js': `require('./deep/c')`,
'deep/c.js': `module.exports = {}`
},
{ server: ['index.js', 'a.js', 'b.js'] } // `deep` missing
)
try {
const { uncovered } = checkDeclaration(root)
assert.ok(uncovered.has('server/deep'))
} finally {
cleanup(root)
}
})
// ── Resolution has to match Node's, or it invents failures ─────────────────
test('resolves a directory to its index.js', () => {
const root = fixture({ 'index.js': `require('./boot')`, 'boot/index.js': `module.exports = {}` },
{ server: ['index.js', 'boot'] })
try {
assert.strictEqual(checkDeclaration(root).uncovered.size, 0)
} finally {
cleanup(root)
}
})
test('resolves a .json dependency, and does not try to parse it for requires', () => {
const root = fixture({ 'index.js': `require('./data/atlas.json')`, 'data/atlas.json': `{"a":1}` },
{ server: ['index.js', 'data'] })
try {
const { uncovered, missing } = checkDeclaration(root)
assert.strictEqual(missing.length, 0)
assert.strictEqual(uncovered.size, 0)
} finally {
cleanup(root)
}
})
test('survives a require cycle', () => {
const root = fixture({ 'index.js': `require('./a')`, 'a.js': `require('./index')` },
{ server: ['index.js', 'a.js'] })
try {
assert.strictEqual(checkDeclaration(root).uncovered.size, 0)
} finally {
cleanup(root)
}
})
test('a specifier that resolves to nothing is reported, not thrown', () => {
const root = fixture({ 'index.js': `require('./gone')` })
try {
const { missing } = checkDeclaration(root)
assert.strictEqual(missing.length, 1)
assert.strictEqual(missing[0].specifier, './gone')
} finally {
cleanup(root)
}
})
test('prose describing a require is not a require', () => {
// The failure mode checkImports.js hit the first time it ran: index.js's own
// header explains why it must never require express, and comments in this
// repo name module paths constantly.
const root = fixture(
{ 'index.js': `// this file used to require('./commands/gone')\nmodule.exports = 1` },
{ server: ['index.js'] }
)
try {
assert.strictEqual(checkDeclaration(root).missing.length, 0)
} finally {
cleanup(root)
}
})
test('node_modules inside a bundle is npm\'s business, not this check\'s', () => {
const root = fixture({
'index.js': `module.exports = 1`,
'node_modules/ws/index.js': `require('./lib/that-npm-owns')`
})
try {
assert.strictEqual(checkBundle(root).missing.length, 0)
} finally {
cleanup(root)
}
})
// ── And the real repo, which is the check that actually gates a release ────
test('the real ci/bundle.json covers everything the real entry point reaches', () => {
const { uncovered, missing, reached } = checkDeclaration()
assert.deepStrictEqual([...uncovered.keys()], [])
assert.deepStrictEqual(missing, [])
assert.ok(reached > 1, 'the walk should reach more than the entry point itself')
})
test('every path ci/bundle.json declares exists', () => {
// A list naming a path that has moved packs nothing and says nothing — `cp`
// in the release would fail, but only after the tag had been pushed.
for (const p of declaredServerPaths()) {
assert.ok(fs.existsSync(p), `ci/bundle.json names ${p}, which does not exist`)
}
})
test('the entry point is reachable from the declared list', () => {
const entry = path.resolve(__dirname, '..', 'index.js')
assert.ok(reachable(entry).files.includes(entry))
assert.ok(resolveFile(path.dirname(entry), './core'))
})