feat(admin): the Modules screen (phase 4, slice 2) #143
Reference in New Issue
Block a user
No description provided.
Delete Branch "feature/module-admin-screen"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The screen website#142's API was written for.
Install from a release URL, enable, disable, uninstall, purge, restart. Admin-only, matching the server — and core's own screen, because it is how a module reaches the volume at all.
Everything that decides what a row says and which buttons it offers lives in
lib/moduleAdmin.js— plain JS so the DOM-less runner can reach it, the same reasonlib/adminNav.jsis. The JSX renders what it returns.Three sources of truth, allowed to disagree
The row records what the operator decided and what the last boot did. The loader says what is mounted and answering. The volume says whether there is a directory at all. Picking one and rendering it is simpler and lies.
The case that makes it concrete is the one decision 3 creates on purpose — disable a module (its
onShutdownruns), then enable it again. The row saysenabled; the loader still saysdisabled, because nothing can start it before a restart. Neither "Running" nor "Disabled" is true:Two shapes deliberately unlike the rest of the panel:
purge.sqllives inside the directory being deleted and there is no later.What the browser found that no test could
Installing over a row the previous boot had left
startup_failedrendered "Failed at the require stage: module directory not present on the volume" one second after the files had been written to the volume — and, because that branch is not pending, it suppressed the restart banner the install had just told the operator to use. Every unit test passed; none had modelled a stale row plus a fresh install.The fix is a derivation, not a special case: the loader scans the volume once at require time, so a module on the volume now with no live record arrived after that scan, and everything the row says about it predates the install. That check runs before the failure one.
Same class, one step on: an upgrade leaves the old code loaded, so the row's version is a promise about the next boot.
liveVersion(added in #142) lets the screen say "Restart to finish upgrading" rather than reporting the new version as running.Verified against a live server and the real release
Pasted the published v0.3.0 install-manifest URL into the form, restarted, and watched it come up:
The module's own nav rows (In-Game Ops, Houses) appeared in the sidebar. Then disable ran its
onShutdownfor real:— the uo-link WebSocket closed, its routes went to 404, and it left the public list. Enable then showed the decision-3 state with the banner.
The restart button was exercised through its endpoint rather than clicked, because a
window.confirmwedges the browser automation. That run is what found the Windows signal defect fixed in #142.AI disclosure
🤖 Generated with Claude Code
Standing the slice-2 screen up against a live server and installing the published module-uo v0.3.0 through it found three things, none of which any unit test in this repo could have caught. Two of them are older than this phase. 1. The boot refresh nulled every install's provenance -------------------------------------------------------- `installed_modules.source` and `.sha256` exist so the admin panel can say where a module came from. They never survived a restart. `lifecycle.boot()` re-records every scanned module with no source and no sha256 -- correctly, because a scan finds a directory and never where it came from -- and `upsert` assigned both columns unconditionally. So an install's provenance lasted exactly until the restart that install asked for, and the screen then described a module installed from a URL as "placed on the volume by hand". Verified live: install, restart, provenance gone. Nothing could have caught it before now. Phase 4 wrote the first non-null value these columns had ever had, so lifecycle.js's comment asserting that "recordInstalled leaves what it is not given" described an intention rather than the statement below it -- and modules.model.test.js's fake reproduced the defect faithfully, assigning unconditionally just like the SQL. Fixed with COALESCE(VALUES(col), col): a value overwrites, a NULL leaves what is there. The fake now matches, and two tests pin both directions -- a boot refresh must not wipe it, and a re-install from a new URL must still replace it, or the column would become write-once and an upgrade would for ever show where the first version came from. 2. The restart killed the server on Windows instead of stopping it ------------------------------------------------------------------ The route called `process.kill(process.pid, 'SIGTERM')` to reach server.js's graceful-shutdown handler. That works on Linux. **Windows has no POSIX signals, and Node documents SIGTERM there as unconditional termination of the target process** -- so on a Windows host the restart killed the server outright: no module onShutdown, no listener close, no pool close, no log flush. Observed exactly that: the process was gone and the shutdown handler had logged nothing at all. `process.on('SIGTERM', ...)` is an ordinary EventEmitter listener, so `process.emit('SIGTERM')` reaches the same handler on every platform without involving the OS. One shutdown path, still; it just gets there by an event. Deployment is Linux containers and would never have shown this. Development is not, and neither is the smoke that found it. The test was worse than useless: it stubbed `process.kill` and asserted it had been called with SIGTERM, which is precisely the call whose MEANING differs by platform. It now waits for the SIGTERM EVENT -- what server.js is actually subscribed to -- so a pass here means the handler would run. 3. `present()` did not publish the running version -------------------------------------------------- An upgrade writes new files and a new row while the old code stays loaded, so the row's version is a promise about the next boot rather than a description of this one. Adds `liveVersion` from the loader beside `liveState`, so the screen can tell the two apart instead of reporting the new version as running. 723 server tests (+2), manifest and OpenAPI both unchanged. Co-Authored-By: Claude <noreply@anthropic.com>