From 6dc65a4434678d38378c650ac27766b0fa60366e Mon Sep 17 00:00:00 2001 From: Mostafa Sadeghi <205455727+mostafasadeghidev@users.noreply.github.com> Date: Sun, 9 Aug 2026 03:45:10 +0200 Subject: [PATCH] fix(plugins): an upgrade no longer 404s every published page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Published HTML links a plugin's frontend assets by version — `/uploads/plugins///frontend/app.js` — because the version is what makes the URL cache-bustable. The upgrade flow deleted the old version's directory as its last step, and nothing re-rendered the artefacts already on disk, so every one of them kept pointing at files that were gone. On a real site an upgrade took out jQuery, GSAP, Lenis, Splide and the boot script across all six pages at once — the entire site's JavaScript, silently. Nothing warned, nothing prompted a re-publish, and the pages still returned 200 with a correct-looking document. The fix was one publish, for someone who already knew that. So the delete belongs at publish, not at upgrade. A publish is the only thing that rewrites those URLs, which makes it the exact moment the old files stop being referenced. `sweepStalePluginVersionAssets` runs after the slot swap and drops every version directory except the installed one. Between an upgrade and the next publish both versions sit on disk: the installed one for new renders, the previous one for pages not yet re-baked. The cost is bounded by how many upgrades happen between two publishes, and each version is a bundle rather than a library. A plugin with no installed record is never swept. Uninstall already removes its tree, so anything still there is unexplained — and a publish is a bad moment to act on something unexplained. Sweep failure is logged and swallowed: leftover files are wasted disk, never a broken page. Rollback still deletes the NEW version's directory, which is correct — no published page has ever referenced it. --- docs/features/plugin-system.md | 4 + server/__tests__/stale-plugin-assets.test.ts | 101 +++++++++++++++++++ server/handlers/cms/plugins/install.ts | 16 +-- server/publish/publishSite.ts | 13 +++ server/publish/stalePluginAssets.ts | 77 ++++++++++++++ 5 files changed, 205 insertions(+), 6 deletions(-) create mode 100644 server/__tests__/stale-plugin-assets.test.ts create mode 100644 server/publish/stalePluginAssets.ts diff --git a/docs/features/plugin-system.md b/docs/features/plugin-system.md index a4e4f94a7..bc14cb8f6 100644 --- a/docs/features/plugin-system.md +++ b/docs/features/plugin-system.md @@ -217,6 +217,10 @@ Upgrade to v2: (old) deactivate → (new) migrate({fromVersion}) → (new) ac Uninstall: (if active) deactivate → uninstall ``` +**An upgrade does not delete the previous version's files.** Published pages link a plugin's frontend assets by version (`/uploads/plugins///frontend/…`, which is what makes the URL cache-bustable), and those artefacts are only rewritten by a **publish**. Deleting on upgrade therefore 404'd every page already baked to disk — on a real site an upgrade took out jQuery, GSAP, Lenis, Splide and the boot script across every page at once, with no warning and no prompt to re-publish. + +The old directory is retired by the next publish instead, which is the exact moment those URLs stop pointing at it (`sweepStalePluginVersionAssets`, `server/publish/stalePluginAssets.ts`, called after the slot swap). Between an upgrade and the next publish both versions sit on disk: the installed one for new renders, the previous one for pages not yet re-baked. A plugin with no installed record is never swept — uninstall already removes its tree, so anything left is unexplained, and a publish is a bad moment to act on that. + Each hook receives the `api` object (see below). All hooks may be sync or async. If any hook throws, the host: 1. Rolls back to the previous lifecycle state. diff --git a/server/__tests__/stale-plugin-assets.test.ts b/server/__tests__/stale-plugin-assets.test.ts new file mode 100644 index 000000000..ebcfd42b2 --- /dev/null +++ b/server/__tests__/stale-plugin-assets.test.ts @@ -0,0 +1,101 @@ +/** + * Retiring plugin versions at the right moment. + * + * Published HTML links a plugin's frontend assets by version, and only a + * publish rewrites those links. Deleting the old version during the UPGRADE + * therefore broke every page already on disk — on a real site it 404'd jQuery, + * GSAP, Lenis, Splide and the boot script across all six pages at once, with + * no warning and no prompt to re-publish. + * + * These pin the two halves: the sweep removes what a fresh publish has stopped + * referencing, and it refuses to touch anything it cannot account for. + */ + +import { describe, expect, test, beforeEach, afterEach } from 'bun:test' +import { mkdir, mkdtemp, rm, writeFile, readdir } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { sweepStalePluginVersionAssets } from '../publish/stalePluginAssets' +import type { DbClient } from '../db/client' + +let uploadsDir = '' + +/** A DbClient stub whose only job is to answer the installed-plugins query. */ +function dbWithInstalled(installed: Array<{ id: string; version: string }>): DbClient { + const rows = installed.map((p) => ({ + id: p.id, + version: p.version, + manifest_json: { id: p.id, name: p.id, version: p.version, apiVersion: 1 }, + enabled: true, + status: 'active', + settings_json: {}, + granted_permissions_json: [], + installed_at: new Date().toISOString(), + updated_at: new Date().toISOString(), + })) + const client = (async () => ({ rows, rowCount: rows.length })) as unknown as DbClient + return client +} + +async function seedVersion(pluginId: string, version: string): Promise { + const dir = join(uploadsDir, 'plugins', pluginId, version, 'frontend') + await mkdir(dir, { recursive: true }) + await writeFile(join(dir, 'app.js'), '// bundle') +} + +const versionsOf = async (pluginId: string): Promise => + (await readdir(join(uploadsDir, 'plugins', pluginId), { withFileTypes: true })) + .filter((e) => e.isDirectory()).map((e) => e.name).sort() + +beforeEach(async () => { uploadsDir = await mkdtemp(join(tmpdir(), 'instatic-sweep-')) }) +afterEach(async () => { await rm(uploadsDir, { recursive: true, force: true }) }) + +describe('sweepStalePluginVersionAssets', () => { + test('removes the superseded version and keeps the installed one', async () => { + await seedVersion('acme.demo', '1.0.0') + await seedVersion('acme.demo', '1.1.0') + const result = await sweepStalePluginVersionAssets(dbWithInstalled([{ id: 'acme.demo', version: '1.1.0' }]), uploadsDir) + expect(result.removed).toBe(1) + expect(await versionsOf('acme.demo')).toEqual(['1.1.0']) + }) + + test('removes several versions when publishes lagged behind upgrades', async () => { + for (const v of ['1.0.0', '1.1.0', '1.2.0', '1.3.0']) await seedVersion('acme.demo', v) + const result = await sweepStalePluginVersionAssets(dbWithInstalled([{ id: 'acme.demo', version: '1.3.0' }]), uploadsDir) + expect(result.removed).toBe(3) + expect(await versionsOf('acme.demo')).toEqual(['1.3.0']) + }) + + test('a single installed version is left alone', async () => { + await seedVersion('acme.demo', '1.0.0') + const result = await sweepStalePluginVersionAssets(dbWithInstalled([{ id: 'acme.demo', version: '1.0.0' }]), uploadsDir) + expect(result.removed).toBe(0) + expect(await versionsOf('acme.demo')).toEqual(['1.0.0']) + }) + + test('a plugin with no installed record is never touched', async () => { + // Uninstall already removes the tree, so anything still here is + // unexplained — and unexplained is a bad reason to delete from a live + // volume during a publish. + await seedVersion('mystery.plugin', '9.9.9') + const result = await sweepStalePluginVersionAssets(dbWithInstalled([]), uploadsDir) + expect(result.removed).toBe(0) + expect(await versionsOf('mystery.plugin')).toEqual(['9.9.9']) + }) + + test('only the named plugin is swept', async () => { + await seedVersion('acme.demo', '1.0.0') + await seedVersion('acme.demo', '1.1.0') + await seedVersion('other.plugin', '2.0.0') + await sweepStalePluginVersionAssets(dbWithInstalled([ + { id: 'acme.demo', version: '1.1.0' }, + { id: 'other.plugin', version: '2.0.0' }, + ]), uploadsDir) + expect(await versionsOf('other.plugin')).toEqual(['2.0.0']) + }) + + test('a site with no plugins directory is not an error', async () => { + const result = await sweepStalePluginVersionAssets(dbWithInstalled([]), uploadsDir) + expect(result.removed).toBe(0) + }) +}) diff --git a/server/handlers/cms/plugins/install.ts b/server/handlers/cms/plugins/install.ts index 5b16ea358..66138ff84 100644 --- a/server/handlers/cms/plugins/install.ts +++ b/server/handlers/cms/plugins/install.ts @@ -352,12 +352,16 @@ async function installUpgradeFromPackage(ctx: UpgradeContext): Promise ) } - // 6. Drop the old version's assets. With worker isolation, plugin server - // files no longer live in the host process's `bun --watch` graph - // (they're imported inside the worker), so deleting them here doesn't - // race the response write — straightforward `await rm` is safe in - // both dev and production. - await removePluginVersionAssets(options.uploadsDir, pluginId, fromVersion) + // 6. The old version's files STAY. Published pages link plugin frontend + // assets by version (`/uploads/plugins///frontend/…`), and + // those artefacts are only rewritten by a publish — so deleting here + // 404'd every page already on disk. On a real site an upgrade took out + // jQuery, GSAP, Lenis, Splide and the boot script across every page at + // once, with no warning and no prompt to re-publish. + // + // The next publish retires them, because that is the moment the URLs + // stop pointing at this version: `sweepStalePluginVersionAssets` in + // `server/publish/stalePluginAssets.ts`. // Re-fetch so the response carries the post-activation row (settings, // lifecycle = 'active', etc.). diff --git a/server/publish/publishSite.ts b/server/publish/publishSite.ts index 0c39dd896..1b5b96a8c 100644 --- a/server/publish/publishSite.ts +++ b/server/publish/publishSite.ts @@ -51,6 +51,7 @@ import { buildPublishedSiteCssBundle } from './siteCssBundle' import { bakePublishedDataRowArtefacts } from './bakeDataRows' import { bumpPublishVersion, getPublishVersion, withPublishLock } from './publishState' import { runPublishFlush } from './publishFlush' +import { sweepStalePluginVersionAssets } from './stalePluginAssets' interface PublishResult { publishedPages: number @@ -298,6 +299,18 @@ async function publishDraftSiteLocked( } catch (err) { console.error('[publish:site] static artefact write failed (live renderer remains active):', err) } + + // The artefacts just written link the CURRENTLY installed plugin versions, + // so any older version's files are now referenced by nothing. This is the + // only moment that is true — which is why an upgrade must not delete them + // itself. Leftovers are wasted disk, never a broken page, so a failure + // here is logged and the publish still succeeds. + try { + const { removed } = await sweepStalePluginVersionAssets(db, uploadsDir) + if (removed > 0) console.error(`[publish:site] retired ${removed} stale plugin version dir(s)`) + } catch (err) { + console.error('[publish:site] stale plugin asset sweep failed (harmless, retries next publish):', err) + } } // Layer B: invalidate the in-memory render cache so the next visitor request diff --git a/server/publish/stalePluginAssets.ts b/server/publish/stalePluginAssets.ts new file mode 100644 index 000000000..bbf2d3ee2 --- /dev/null +++ b/server/publish/stalePluginAssets.ts @@ -0,0 +1,77 @@ +/** + * Retire a plugin version's files only once nothing published points at them. + * + * Published HTML links a plugin's frontend assets by version — + * `/uploads/plugins///frontend/app.js` — because the version is + * what makes the URL cache-bustable. Upgrading used to delete the old version's + * directory immediately, which broke every page already on disk: the artefacts + * still carried the old path, and nothing re-rendered them. On a real site an + * upgrade 404'd jQuery, GSAP, Lenis, Splide and the boot script on all six + * pages at once — the whole site's JavaScript, with no warning and no prompt + * to re-publish. A publish fixed it, but only for someone who already knew. + * + * So the delete belongs at publish, not at upgrade. Publish is the only thing + * that rewrites those URLs, which makes it the exact moment the old files stop + * being referenced. Between an upgrade and the next publish both versions sit + * on disk: the installed one for new renders, the previous one for pages that + * have not been re-baked yet. The cost is bounded by how many upgrades happen + * between two publishes, and each version is a bundle, not a library. + * + * `publishSite.ts` calls this after the slot swap. Failure is logged and + * swallowed — leftover files are wasted disk, never a broken page, and a + * publish must not fail over cleanup. + */ + +import { readdir, rm } from 'node:fs/promises' +import { join } from 'node:path' +import type { DbClient } from '../db/client' +import { listInstalledPlugins } from '../repositories/plugins' + +/** + * Delete every plugin version directory except the installed one. + * + * A plugin with no installed record is left entirely alone: uninstall already + * removes its tree, so anything still here is unexplained, and unexplained is + * not a good reason to delete from a live volume. + */ +export async function sweepStalePluginVersionAssets( + db: DbClient, + uploadsDir: string, +): Promise<{ removed: number }> { + const pluginsRoot = join(uploadsDir, 'plugins') + + const currentVersion = new Map() + for (const result of await listInstalledPlugins(db)) { + if (result.kind !== 'ok') continue + currentVersion.set(result.plugin.id, result.plugin.version) + } + + let removed = 0 + let pluginDirs: string[] + try { + pluginDirs = (await readdir(pluginsRoot, { withFileTypes: true })) + .filter((e) => e.isDirectory()) + .map((e) => e.name) + } catch { + return { removed: 0 } // no plugins installed on this site + } + + for (const pluginId of pluginDirs) { + const keep = currentVersion.get(pluginId) + if (!keep) continue + let versions: string[] + try { + versions = (await readdir(join(pluginsRoot, pluginId), { withFileTypes: true })) + .filter((e) => e.isDirectory()) + .map((e) => e.name) + } catch { + continue + } + for (const version of versions) { + if (version === keep) continue + await rm(join(pluginsRoot, pluginId, version), { recursive: true, force: true }) + removed += 1 + } + } + return { removed } +}