feat(modules): boot/shutdown hook dispatch and the installed_modules reconcile #132

Merged

whitlocktech merged 2 commits from feature/module-lifecycle into edge 2026-08-11 02:34:49 +00:00

Conversation 0 Commits 2 Files Changed 10 +861 -53

wtclaude commented 2026-08-11 01:33:02 +00:00

Member

Copy Link

Phase 2 PR 5 of docs/website/MODULE_SYSTEM.md §2.7. api.onBoot / api.onShutdown stop throwing, server.js gains one call on each side, and the §2.4 state machine finally runs against real outcomes — which is what makes §4.5's disabled 404 leg reachable for the first time. Docs half: docs#130.

Nothing an operator or a client can see changes. 901 tests pass (17 new), routes.manifest.json is unchanged at 229 routes, and the OpenAPI spec regenerates byte-identical.

Carries a second, unrelated commit: the test suite never terminated, and that turned out to be a live-database bug rather than slowness. Detail at the bottom.

Where it lives, and why not in the loader

server/src/modules/lifecycle.js, for the same reason modules/schema.js exists: scripts/routeManifest.js and swagger/swagger.js both require app.js with the pool pointed at a dead port (§4.1), so the loader may not reach the database. This half is database-first.

The two halves meet at exactly one function — loader.setState() — so the in-memory record the dispatch guard reads and the row the admin panel reads are moved together and cannot drift apart.

The four decisions

The loader classifies its failures	Every failure is now recorded against the §4.3 step that produced it, so failure_stage says where a module broke instead of being a column nothing ever filled. The four steps readManifest covers in one pass label themselves; the rest are inferred from how far load() had got, and an unlabelled throw is recorded against the step that was running rather than guessed at.
A row whose directory is gone is marked failed	The boot reset has just moved it to enabled, and a row claiming to be enabled for a module that is not on the volume is the one state that is simply untrue — read that way by the admin panel and by GET /api/v1/public/modules alike. An uninstall leaves disabled, which the reset never touches, so this catches only a hand-deleted directory.
Core's eight UO boot call sites stay in server.js	Until Phase 3. PR 4's registries had to move now because a registered announce leg had nowhere to live; a boot call site already has somewhere to live, so moving it now would be extraction done early — behaviour change in a phase whose exit criterion is that nothing changes.
onBoot gets no timeout	Shutdown races the process being killed; boot does not. A slow onBoot delaying the listener is the contract's promise to a module that must warm up before it serves, not a problem to time out. onShutdown keeps its 5s budget.

What a boot does, in order

1. Clear the last boot's outcomes, so what is on display afterwards is what this boot did. disabled rows are left alone — a decision, not an outcome.
2. Write a row for every module on the volume, with null provenance if it has none: a hand-placed directory is a supported install, and without a row it could be neither disabled nor reported.
3. Mark any row whose directory is gone startup_failed.
4. Write down the outcome each module already carries — disabled, or failed during load or schema replay (both of which happen before the database is reachable) — and only then dispatch onBoot.

Two rules fall out and are tested:

• The operator's switch wins over everything, including a failure. A disabled module is guarded, is not booted, and does not have its failure re-recorded — overwriting a decision with an outcome would silently switch it back on next boot.
• A bookkeeping failure is not a boot failure. Every database write in the reconcile is individually caught. A row that will not update is worse reporting; it must never stop the modules behind it from booting, let alone the site from starting.

Second commit: the suite was talking to a real database

npm test did not terminate, and the reason was not slowness. Twenty-two test files omitted the two lines that point the pool at a dead port, so utils/db.js — which builds its mariadb pool at require time and calls dotenv.config() itself — picked up server/.env and opened five live connections to the developer's MariaDB. The tests still passed, because they stub their models and never issue a query, so the only symptoms were a process that never exited and five connections held for as long as it lived. Thirty stranded workers is 150 connections, which is the whole server's limit — the "too many connections" this workspace has hit before.

The convention was right and only ever as good as the next test file's memory of it, so it moves into the harness:

• test/_setup.js, loaded with --require from the npm script, ahead of the test file it hosts — the only moment early enough, since the pool is built at require time. It pins the dead port (dotenv does not overwrite an existing variable, so an explicit DB_PORT= still wins for anyone who wants a live database) and closes the pool after that file's tests, so the process exits at once rather than waiting out the driver's connect retries. The per-file preambles stay: they keep node --test test/one.test.js safe on its own.
• db.close() is idempotent. pool.end() throws on a second call, and closing twice is now normal rather than exceptional — the harness closes on top of the suites that close themselves, and a SIGINT followed by a SIGTERM already reached the shutdown handler twice.
• test/_helper.js's close() destroys open connections. server.close() only stops accepting and waits for existing connections to end, and node's global fetch keeps its sockets alive, so the listener outlived the test that created it — invisible until now, because the pool was holding the process open anyway.

announceJobs.test.js alone: a 120s+ hang → 0.35s.

Verification

• 901 tests, 901 pass, ~75s — and it finishes, which it did not before.
• Verified three times on CI's exact platform: node:20 on Linux via Docker, 901/901 each time.
• 13 new tests in moduleLifecycle.test.js (reconcile order, hand-placed rows, the 503 reached by a real failing onBoot, the 404 reached by a real disabled row, orphan rows, a database that refuses every write, a hung hook, a thrown hook, reverse shutdown order) and 4 in moduleLoader.test.js (hook validation, the stage table, bootable/shutdownHooks).
• routes.manifest.json: zero-line diff, 227 public + 2 internal. routes.guards.json unchanged. npm run swagger regenerates byte-identical — PR 4's slot merge still finds all six paths.

One honest caveat: on Windows + Node 24 a very fast test file still occasionally reports a bare 'test failed' with no diagnostics, roughly once in fourteen runs, a different file each time and never reproducible in isolation. It does not occur on the CI platform across the runs above, and five instrumented runs caught no uncaught exception or unhandled rejection behind it — it looks like a runner-level race on children that now exit promptly, so I have not papered over it.

AI-assisted (Claude Code).

🤖 Generated with Claude Code

https://claude.ai/code/session_018ocYxQWk3EhZe5gWRJXFU8

Phase 2 PR 5 of `docs/website/MODULE_SYSTEM.md` §2.7. `api.onBoot` / `api.onShutdown` stop throwing, `server.js` gains one call on each side, and the §2.4 state machine finally runs against real outcomes — which is what makes §4.5's `disabled` 404 leg reachable for the first time. Docs half: **docs#130**. **Nothing an operator or a client can see changes.** 901 tests pass (17 new), `routes.manifest.json` is unchanged at 229 routes, and the OpenAPI spec regenerates byte-identical. Carries a second, unrelated commit: **the test suite never terminated**, and that turned out to be a live-database bug rather than slowness. Detail at the bottom. ## Where it lives, and why not in the loader `server/src/modules/lifecycle.js`, for the same reason `modules/schema.js` exists: `scripts/routeManifest.js` and `swagger/swagger.js` both require `app.js` with the pool pointed at a dead port (§4.1), so the loader may not reach the database. This half is database-first. The two halves meet at **exactly one function** — `loader.setState()` — so the in-memory record the dispatch guard reads and the row the admin panel reads are moved together and cannot drift apart. ## The four decisions | | | |---|---| | **The loader classifies its failures** | Every failure is now recorded against the §4.3 step that produced it, so `failure_stage` says *where* a module broke instead of being a column nothing ever filled. The four steps `readManifest` covers in one pass label themselves; the rest are inferred from how far `load()` had got, and an unlabelled throw is recorded against the step that was running rather than guessed at. | | **A row whose directory is gone is marked failed** | The boot reset has just moved it to `enabled`, and a row claiming to be enabled for a module that is not on the volume is the one state that is simply untrue — read that way by the admin panel and by `GET /api/v1/public/modules` alike. An uninstall leaves `disabled`, which the reset never touches, so this catches only a hand-deleted directory. | | **Core's eight UO boot call sites stay in `server.js`** | Until Phase 3. PR 4's registries had to move now because a registered announce leg had nowhere to live; a boot call site already has somewhere to live, so moving it now would be extraction done early — behaviour change in a phase whose exit criterion is that nothing changes. | | **`onBoot` gets no timeout** | Shutdown races the process being killed; boot does not. A slow `onBoot` delaying the listener is the contract's *promise* to a module that must warm up before it serves, not a problem to time out. `onShutdown` keeps its 5s budget. | ## What a boot does, in order 1. Clear the last boot's outcomes, so what is on display afterwards is what *this* boot did. `disabled` rows are left alone — a decision, not an outcome. 2. Write a row for every module on the volume, with null provenance if it has none: a hand-placed directory is a supported install, and without a row it could be neither disabled nor reported. 3. Mark any row whose directory is gone `startup_failed`. 4. Write down the outcome each module already carries — disabled, or failed during load or schema replay (both of which happen before the database is reachable) — and only then dispatch `onBoot`. Two rules fall out and are tested: - **The operator's switch wins over everything, including a failure.** A `disabled` module is guarded, is not booted, and does **not** have its failure re-recorded — overwriting a decision with an outcome would silently switch it back on next boot. - **A bookkeeping failure is not a boot failure.** Every database write in the reconcile is individually caught. A row that will not update is worse reporting; it must never stop the modules behind it from booting, let alone the site from starting. ## Second commit: the suite was talking to a real database `npm test` did not terminate, and the reason was not slowness. **Twenty-two test files omitted the two lines that point the pool at a dead port**, so `utils/db.js` — which builds its mariadb pool at *require* time and calls `dotenv.config()` itself — picked up `server/.env` and opened **five live connections to the developer's MariaDB**. The tests still passed, because they stub their models and never issue a query, so the only symptoms were a process that never exited and five connections held for as long as it lived. Thirty stranded workers is 150 connections, which is the whole server's limit — the "too many connections" this workspace has hit before. The convention was right and only ever as good as the next test file's memory of it, so it moves into the harness: - **`test/_setup.js`**, loaded with `--require` from the npm script, ahead of the test file it hosts — the only moment early enough, since the pool is built at require time. It pins the dead port (`dotenv` does not overwrite an existing variable, so an explicit `DB_PORT=` still wins for anyone who wants a live database) and closes the pool after that file's tests, so the process exits at once rather than waiting out the driver's connect retries. The per-file preambles stay: they keep `node --test test/one.test.js` safe on its own. - **`db.close()` is idempotent.** `pool.end()` throws on a second call, and closing twice is now normal rather than exceptional — the harness closes on top of the suites that close themselves, and a SIGINT followed by a SIGTERM already reached the shutdown handler twice. - **`test/_helper.js`'s `close()` destroys open connections.** `server.close()` only stops accepting and waits for existing connections to end, and node's global `fetch` keeps its sockets alive, so the listener outlived the test that created it — invisible until now, because the pool was holding the process open anyway. `announceJobs.test.js` alone: a 120s+ hang → **0.35s**. ## Verification - **901 tests, 901 pass, ~75s** — and it finishes, which it did not before. - **Verified three times on CI's exact platform**: `node:20` on Linux via Docker, 901/901 each time. - **13 new tests in `moduleLifecycle.test.js`** (reconcile order, hand-placed rows, the 503 reached by a real failing `onBoot`, the 404 reached by a real `disabled` row, orphan rows, a database that refuses every write, a hung hook, a thrown hook, reverse shutdown order) and 4 in `moduleLoader.test.js` (hook validation, the stage table, `bootable`/`shutdownHooks`). - **`routes.manifest.json`: zero-line diff**, 227 public + 2 internal. `routes.guards.json` unchanged. **`npm run swagger` regenerates byte-identical** — PR 4's slot merge still finds all six paths. One honest caveat: on Windows + Node 24 a very fast test file still occasionally reports a bare `'test failed'` with no diagnostics, roughly once in fourteen runs, a different file each time and never reproducible in isolation. It does not occur on the CI platform across the runs above, and five instrumented runs caught no uncaught exception or unhandled rejection behind it — it looks like a runner-level race on children that now exit promptly, so I have not papered over it. AI-assisted (Claude Code). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_018ocYxQWk3EhZe5gWRJXFU8

wtclaude added 1 commit 2026-08-11 01:33:03 +00:00

feat(modules): boot/shutdown hook dispatch and the installed_modules reconcile ... 21196466ed

Phase 2, PR 5 of docs/website/MODULE_SYSTEM.md 2.7. api.onBoot/api.onShutdown
stop throwing, server.js gains one call on each side, and the 2.4 state machine
finally runs against real outcomes -- which is what makes 4.5's `disabled` 404
leg reachable for the first time.

Dispatch and reconcile live in src/modules/lifecycle.js rather than in the
loader, for the reason the schema replay does: routeManifest.js and swagger.js
both require app.js against a dead pool, so the loader may not reach the
database. The two halves meet at exactly one function, loader.setState(), so the
in-memory record the dispatch guard reads and the row the admin panel reads are
moved together and cannot disagree.

Four decisions, all recorded in MODULE_API.md 2.5 and 4.4:

- The loader classifies its failures by 4.3 step, so failure_stage says where a
  module broke instead of being a column nothing ever filled. The four steps
  readManifest covers in one pass label themselves; the rest are inferred from
  how far load() had got, and an unlabelled throw is recorded against the step
  that was running rather than guessed at.
- A row whose directory is gone is marked startup_failed rather than left
  claiming `enabled` -- the boot reset has just moved it there, and a row
  claiming to be enabled for a module that is not on the volume is the one state
  that is simply untrue. An uninstall leaves `disabled`, which the reset never
  touches, so this catches only a hand-deleted directory.
- Core's eight UO boot call sites stay in server.js until Phase 3. Unlike a
  registered announce leg, a boot call site already has somewhere to live, so
  moving it now would be extraction done early in a phase whose exit criterion
  is that nothing changes.
- onBoot gets no timeout. Shutdown races a SIGKILL and boot does not, and a slow
  onBoot delaying the listener is the contract's promise to a module that must
  warm up before it serves.

The operator's switch wins over everything: a disabled module is guarded, not
booted, and does not have its failure re-recorded, or an outcome would silently
switch it back on next boot. Every database write in the reconcile is
individually caught -- a row that will not update is worse reporting, never a
failed boot.

900 tests pass (17 new). routes.manifest.json is unchanged at 229 routes and the
OpenAPI spec regenerates byte-identical.

Co-Authored-By: Claude <noreply@anthropic.com>

wtclaude added 1 commit 2026-08-11 02:27:53 +00:00

fix(test): stop the suite reaching a real database, and make it exit ... 32ed8e4411

`npm test` never terminated. Twenty-two test files omitted the two lines that
point the pool at a dead port, so utils/db.js -- which builds its mariadb pool at
require time and calls dotenv.config() itself -- picked up server/.env and opened
five live connections to the developer's MariaDB. The tests still passed, because
they stub their models and never issue a query; the only symptoms were a process
that never exited and five connections held for as long as it lived. Thirty
stranded workers is 150 connections, which is the whole server's limit, and that
is the "too many connections" this workspace has hit before.

The convention was right and only ever as good as the next test file's memory of
it, so it moves into the harness: test/_setup.js is loaded with --require by the
npm script, ahead of the test file it hosts, which is the only moment early
enough to matter. It pins the dead port -- dotenv does not overwrite an existing
variable, so an explicit DB_PORT= still wins for anyone who wants a live database
-- and closes the pool after the file's tests, so the process exits at once
instead of waiting out the driver's connect retries. The per-file preambles stay:
they keep `node --test test/one.test.js` safe on its own.

Two supporting fixes:

- db.close() is idempotent. pool.end() throws "pool is already closed" on a
  second call, and closing twice is now normal rather than exceptional -- the
  harness closes the pool for every file on top of the suites that close it
  themselves, and a SIGINT followed by a SIGTERM already reached the shutdown
  handler twice.
- test/_helper.js's close() destroys open connections. server.close() only stops
  accepting and waits for existing connections to end, and node's global fetch
  keeps its sockets alive, so the listener outlived the test that created it --
  invisible until now, because the pool was holding the process open anyway.

announceJobs.test.js alone: 120s+ hang -> 0.35s. The whole suite now finishes in
~75s where it previously did not finish at all: 901 tests, 901 pass, verified
three times on CI's exact platform (node:20 on Linux, via Docker).

Co-Authored-By: Claude <noreply@anthropic.com>

whitlocktech approved these changes 2026-08-11 02:33:21 +00:00

whitlocktech merged commit 85f563fc16 into edge 2026-08-11 02:34:49 +00:00

whitlocktech deleted branch feature/module-lifecycle 2026-08-11 02:34:49 +00:00

whitlocktech referenced this issue from a commit 2026-08-11 02:34:51 +00:00

Merge pull request 'feat(modules): boot/shutdown hook dispatch and the installed_modules reconcile' (#132) from feature/module-lifecycle into edge

Sign in to join this conversation.

Reviewers

No Reviewers

whitlocktech

Labels

Clear labels

bug

Something is not working

duplicate

This issue or pull request already exists

enhancement

New feature

help wanted

Need some help

invalid

Something is wrong

question

More information is needed

severity:critical

Security audit: critical severity

severity:high

Security audit: high severity

severity:low

Security audit: low severity

severity:medium

Security audit: medium severity

wontfix

This won't be fixed

bug

Something is not working

duplicate

This issue or pull request already exists

enhancement

New feature

help wanted

Need some help

invalid

Something is wrong

question

More information is needed

wontfix

This won't be fixed

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

ksfixitman whitlocktech wtclaude

No Assignees

2 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: RunicGateway/website#132

No description provided.