fix(release): ship server/commands, and check that the bundle is complete #18

wtclaude · 2026-08-19T18:26:40Z

wtclaude commented

2026-08-19 18:26:40 +00:00

What & why

v1.0.0 is broken on install. It unpacks fine and then dies at the register stage on every boot:

ERROR [modules] module "uo" failed to load — continuing without it
  {"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. The bundle shipped without it, so the module is dead on the operator's box.

Why nothing caught it

This 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 that subset exists nowhere except the release. CI was green on a tree that had the file while the artifact did not.

The pre-publish check in release.yml didn't catch it either: it stats the paths module.json declares (server, schema, purge, client.entry, the fragment), and a file reached only by a require inside register() is named in none of them. It passed on a bundle that could not load.

The fix

The include list stays an include list — release.yml's header makes that case (an exclude list ships whatever it forgot) 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 private copy.
server/scripts/checkBundle.js (new, wired into PR checks as check:bundle) asks whether that list still covers everything server/index.js reaches — following requires transitively and through function bodies.

And the release gains a real loadability check: checkBundle.js --bundle <dir> 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 since moved.

Why not just require() the entry point: index.js puts its requires inside register() on purpose — require order is load-bearing, core.init(ctx) has to run before anything under router/ is required. So requiring the entry evaluates exactly one line (require('./core')) and reports success on a bundle missing every router it has. Calling register() for real would need a fake ctx complete enough to satisfy the module, which is what test/ is for — and test/ doesn't ship. Walking the requires statically asks the same question without needing either.

How it was tested

Both modes were run against the real defect, not only against fixtures — commands was removed from the list and from an assembled bundle, and each mode was confirmed to fail and then to pass again:

	`commands` present	`commands` removed
`check:bundle` (source tree)	`OK — ships every file server/index.js reaches (57 files)`	exit 1, names `server/commands`
`--bundle` (assembled bundle)	`OK — every relative require resolves (56 files scanned)`	exit 1, `requires "./commands/guild.command" — not in the bundle`

The --bundle run was against a bundle assembled locally by replaying the workflow's new jq-driven copy steps, so the assembly change is exercised too.

Also run locally, all green:

npm test --prefix server           # 450 tests pass (13 new in test/checkBundle.test.js)
npm run check:imports --prefix server
npm run check:bundle --prefix server

After merge

The release engine derives the version from commit subjects, so this fix: cuts v1.0.1 automatically. Reinstall from the new module-uo-1.0.1.json on Admin → Modules.

Two things I deliberately did not touch

frozen-manifest installs the module by tarring the whole tree. Making it assemble per ci/bundle.json would prove that what ships registers against a real core — strictly stronger than the static check added here. It's a change to a delicate job and beyond this fix's scope, so it is flagged rather than done.
check:swagger reports STALE on a Windows working tree. A local artifact of core.autocrlf=true (the generator writes LF, the working tree holds CRLF), not real staleness — regenerating produces a zero-byte diff and CI on Linux passes. Pre-existing on main; left alone.

Checklist

I have read CONTRIBUTING.md.
The change builds and existing tests/checks pass locally.
I have added or updated tests/docs where it makes sense.
My commits are reasonably scoped with clear messages.

AI-assisted contributions (required)

No AI tools were used to produce this contribution.
AI tools were used. Tool(s): Claude Code (Opus 5). I have reviewed and understand
every change, and take responsibility for it. AI-authored commits are
marked with a Co-Authored-By / Assisted-By trailer.

License

I agree that my contribution is licensed under this project's license
(GNU GPL v3.0 or later), and I have the right to contribute it.

## What & why **v1.0.0 is broken on install.** It unpacks fine and then dies at the register stage on every boot: ``` ERROR [modules] module "uo" failed to load — continuing without it {"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. The bundle shipped without it, so the module is dead on the operator's box. ### Why nothing caught it This 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 that subset exists nowhere except the release. CI was green on a tree that had the file while the artifact did not. The pre-publish check in `release.yml` didn't catch it either: it stats the paths `module.json` declares (`server`, `schema`, `purge`, `client.entry`, the fragment), and a file reached only by a require **inside `register()`** is named in none of them. It passed on a bundle that could not load. ### The fix The include list stays an include list — `release.yml`'s header makes that case (an exclude list ships whatever it forgot) 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 private copy. - **`server/scripts/checkBundle.js`** (new, wired into PR checks as `check:bundle`) asks whether that list still covers everything `server/index.js` reaches — following requires transitively and through function bodies. And the release gains a real loadability check: `checkBundle.js --bundle <dir>` 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 since moved. **Why not just `require()` the entry point:** `index.js` puts its requires inside `register()` on purpose — require order is load-bearing, `core.init(ctx)` has to run before anything under `router/` is required. So requiring the entry evaluates exactly one line (`require('./core')`) and reports success on a bundle missing every router it has. Calling `register()` for real would need a fake `ctx` complete enough to satisfy the module, which is what `test/` is for — and `test/` doesn't ship. Walking the requires statically asks the same question without needing either. ## How it was tested Both modes were run against the **real defect**, not only against fixtures — `commands` was removed from the list and from an assembled bundle, and each mode was confirmed to fail and then to pass again: | | `commands` present | `commands` removed | |---|---|---| | `check:bundle` (source tree) | `OK — ships every file server/index.js reaches (57 files)` | exit 1, names `server/commands` | | `--bundle` (assembled bundle) | `OK — every relative require resolves (56 files scanned)` | exit 1, `requires "./commands/guild.command" — not in the bundle` | The `--bundle` run was against a bundle assembled locally by replaying the workflow's new `jq`-driven copy steps, so the assembly change is exercised too. Also run locally, all green: ```bash npm test --prefix server # 450 tests pass (13 new in test/checkBundle.test.js) npm run check:imports --prefix server npm run check:bundle --prefix server ``` ## After merge The release engine derives the version from commit subjects, so this `fix:` cuts **v1.0.1** automatically. Reinstall from the new `module-uo-1.0.1.json` on Admin → Modules. ## Two things I deliberately did not touch 1. **`frozen-manifest` installs the module by tarring the whole tree.** Making it assemble per `ci/bundle.json` would prove that *what ships* registers against a real core — strictly stronger than the static check added here. It's a change to a delicate job and beyond this fix's scope, so it is flagged rather than done. 2. **`check:swagger` reports STALE on a Windows working tree.** A local artifact of `core.autocrlf=true` (the generator writes LF, the working tree holds CRLF), not real staleness — regenerating produces a zero-byte diff and CI on Linux passes. Pre-existing on `main`; left alone. ## Checklist - [x] I have read [CONTRIBUTING.md](CONTRIBUTING.md). - [x] The change builds and existing tests/checks pass locally. - [x] I have added or updated tests/docs where it makes sense. - [x] My commits are reasonably scoped with clear messages. ## AI-assisted contributions (required) - [ ] No AI tools were used to produce this contribution. - [x] AI tools were used. Tool(s): `Claude Code (Opus 5)`. I have reviewed and understand every change, and take responsibility for it. AI-authored commits are marked with a `Co-Authored-By` / `Assisted-By` trailer. ## License - [x] I agree that my contribution is licensed under this project's license (**GNU GPL v3.0 or later**), and I have the right to contribute it.

wtclaude added 1 commit 2026-08-19 18:26:42 +00:00

fix(release): ship server/commands, and check that the bundle is complete

PR Checks / client-build (pull_request) Successful in 16s

Details

PR Checks / frozen-manifest (pull_request) Successful in 40s

Details

PR Checks / server-tests (pull_request) Successful in 8m38s

Details

3c179e3338

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

whitlocktech merged commit 956e3fb0b4 into main

2026-08-19 18:27:36 +00:00

whitlocktech deleted branch fix/bundle-ships-commands

2026-08-19 18:27:39 +00:00

whitlocktech referenced this issue from a commit

2026-08-19 18:27:39 +00:00

Merge pull request 'fix(release): ship server/commands, and check that the bundle is complete' (#18) from fix/bundle-ships-commands into main

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: RunicGateway/Module-uo#18