fix(modules): stop a module before purging its tables
Uninstall-with-purge ran purge.sql while the module was still started: the tables went, and the module kept serving and ingesting against a schema that no longer existed until lifecycle.stop() finished — up to the five-second hook budget. For module-uo that is the uo-link WebSocket writing shard events into dropped tables, and requests in flight answering 500 where a stopped module answers 404. Nothing required the old order. The comment justified it as "purge while the SQL is still readable", but removeDir is the only step that touches the filesystem, so purge.sql stays readable until after the stop. The 400 for a module that ships no purge.sql is now resolved before anything is stopped, so a refused request leaves the module exactly as it found it. Found while proving Phase 4's acceptance criterion 2 against the real module-uo v0.3.0 release on an empty database (MODULE_SYSTEM.md §2.7.2). 742 server tests (+1); routes.manifest.json and swagger-output.json byte-identical. AI disclosure: this contribution was AI-assisted (Claude Code). Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -11,9 +11,11 @@
|
||||
// - **enable must not touch the loader.** Disable ran the module's onShutdown;
|
||||
// there is no onBoot re-dispatch, so flipping the record back would put a
|
||||
// module with closed sockets and cleared timers back on the nav.
|
||||
// - **purge must run before the directory is removed.** purge.sql lives inside
|
||||
// that directory. Reorder those two lines and the feature silently stops
|
||||
// working, with a 200 and no data deleted.
|
||||
// - **uninstall's four steps have exactly one valid order**: stop, purge,
|
||||
// remove the directory, remove the row. purge.sql lives inside that
|
||||
// directory, so purging after the delete silently does nothing with a 200 —
|
||||
// and purging before the stop drops the tables under a module that is still
|
||||
// running. See the test itself for why neither is visible in a response.
|
||||
//
|
||||
// Point the DB at a closed port BEFORE requiring anything that builds the pool.
|
||||
process.env.DB_HOST = '127.0.0.1'
|
||||
@@ -333,10 +335,20 @@ test('a shutdown hook that failed is reported rather than swallowed', async () =
|
||||
|
||||
// ── uninstall and purge ────────────────────────────────────────────────────
|
||||
|
||||
test('uninstall purges BEFORE it removes the directory', async () => {
|
||||
// The ordering that makes decision 5 work at all: purge.sql is a file inside
|
||||
// the directory being deleted. Swap these two and the endpoint still answers
|
||||
// 200 and deletes nothing.
|
||||
test('uninstall stops the module, THEN purges, THEN removes the directory', async () => {
|
||||
// Two orderings pinned by one assertion, because both are one line away from
|
||||
// being wrong and neither failure is visible in the response:
|
||||
//
|
||||
// - purge BEFORE removeDir, or decision 5 stops working entirely — purge.sql
|
||||
// is a file inside the directory being deleted, and the endpoint would
|
||||
// still answer 200 having deleted nothing.
|
||||
// - stop BEFORE purge, or the tables are dropped under a module that is
|
||||
// still started. It keeps serving and ingesting against a schema that no
|
||||
// longer exists for as long as its onShutdown takes (module-uo's uo-link
|
||||
// WebSocket keeps writing shard events), and requests in flight answer 500
|
||||
// where a stopped module answers 404. Nothing about the old order was
|
||||
// required: purge.sql stays readable until removeDir, which is the only
|
||||
// step that touches the filesystem.
|
||||
const order = []
|
||||
modules.get = async (id) => ({ id, state: 'started' })
|
||||
install.isInstalled = () => true
|
||||
@@ -349,12 +361,31 @@ test('uninstall purges BEFORE it removes the directory', async () => {
|
||||
const res = mockRes()
|
||||
await ctrl.remove(req({ params: { id: 'uo' }, query: { purge: 'true' } }), res)
|
||||
|
||||
assert.deepEqual(order, ['purge', 'stop', 'removeDir', 'removeRow'])
|
||||
assert.deepEqual(order, ['stop', 'purge', 'removeDir', 'removeRow'])
|
||||
assert.equal(res.body.purged, 12)
|
||||
assert.equal(res.body.restartRequired, true)
|
||||
assert.equal(logged[0].action, 'module.purge')
|
||||
})
|
||||
|
||||
test('a refused purge does not stop the module on its way out', async () => {
|
||||
// The 400 branch below is resolved BEFORE anything is stopped. A request that
|
||||
// is going to be refused must leave the module exactly as it found it —
|
||||
// otherwise "you cannot delete this module's data" would silently take the
|
||||
// module down as a side effect of saying no.
|
||||
const order = []
|
||||
modules.get = async (id) => ({ id, state: 'started' })
|
||||
install.isInstalled = () => true
|
||||
install.purgeFile = () => null
|
||||
lifecycle.stop = async () => { order.push('stop'); return { stopped: true, error: null } }
|
||||
install.removeDir = async () => { order.push('removeDir'); return true }
|
||||
|
||||
const res = mockRes()
|
||||
await ctrl.remove(req({ params: { id: 'uo' }, query: { purge: 'true' } }), res)
|
||||
|
||||
assert.equal(res.statusCode, 400)
|
||||
assert.deepEqual(order, [])
|
||||
})
|
||||
|
||||
test('a plain uninstall keeps the row and does not purge', async () => {
|
||||
const order = []
|
||||
modules.get = async (id) => ({ id, state: 'started' })
|
||||
|
||||
Reference in New Issue
Block a user