From a667377ec95d387d156adf5cffd6dba3476b97c8 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 14:00:41 +0000 Subject: [PATCH 1/7] fix(metadata): MetadataPlugin releases its repository from destroy(), not stop() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [#10772] `MetadataPlugin.start()` attaches a real `FileSystemRepository` (armed chokidar watcher + reconciliation sweep), hands it to the `NodeMetadataManager`, and may attach an artifact file watcher on top. The teardown that closed all three was spelled `stop = async (ctx) => …`. `Plugin` (`@objectstack/core`'s `types.ts`) declares `init()`, `start?(ctx)` and `destroy?()` — and no `stop()` — and `ObjectKernel.performShutdown()` / `LiteKernel.destroy()` walk the plugins in reverse calling `plugin.destroy()`. Nothing in the repo calls `stop()` on a plugin, so every one of those handles was still held after `await kernel.shutdown()` had RESOLVED. The alias is an arrow PROPERTY, which is why a method-only reading of the class missed it and #10371's enumeration came out short by five. Repair is the PR-10375 shape: the body moves into `destroy()`, and `stop()` stays as a delegating alias — public API of an exported class, and an embedder may have learned to call it directly precisely BECAUSE the kernel never did. It stays an arrow property so a detached `const { stop } = plugin` call keeps working, and its parameter becomes optional and ignored: `destroy()` takes no context, so teardown logs through the context captured in `init()`. Baseline: the `MetadataPlugin` line is deleted from `KNOWN_TEARDOWN_UNREACHED` in `scripts/check-plugin-teardown-shape.mjs`, on that gate's own stale-entry verdict rather than by line number. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx --- ...lugin-shutdown-releases-repository.test.ts | 175 ++++++++++++++++++ packages/metadata/src/plugin.ts | 42 ++++- scripts/check-plugin-teardown-shape.mjs | 1 - 3 files changed, 215 insertions(+), 3 deletions(-) create mode 100644 packages/metadata/src/plugin-shutdown-releases-repository.test.ts diff --git a/packages/metadata/src/plugin-shutdown-releases-repository.test.ts b/packages/metadata/src/plugin-shutdown-releases-repository.test.ts new file mode 100644 index 0000000000..9e49783a51 --- /dev/null +++ b/packages/metadata/src/plugin-shutdown-releases-repository.test.ts @@ -0,0 +1,175 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#10772] `await kernel.shutdown()` must actually reach `MetadataPlugin`'s + * teardown. + * + * THE DEFECT. `MetadataPlugin.start()` attaches a real `FileSystemRepository` + * (an armed chokidar watcher plus a reconciliation sweep), hands it to the + * `NodeMetadataManager`, and may attach an artifact file watcher on top. The + * teardown that closed all three was spelled `stop = async (ctx) => …`. + * `Plugin` (`@objectstack/core`'s `types.ts`) declares `init()`, + * `start?(ctx)` and `destroy?()` — and NO `stop()` — so + * `ObjectKernel.performShutdown()` and `LiteKernel.destroy()`, which walk the + * plugins in reverse calling `plugin.destroy()`, walked straight past it. + * Nothing in the repo ever called `stop()` on a plugin. + * + * WHY THE #10371 CENSUS MISSED IT. The alias is an arrow PROPERTY, not a + * method, so a method-only reading of the class does not see it at all. That + * is the whole reason this member — and `AppPlugin` and + * `ExternalValidationPlugin` — were absent from an enumeration that was + * otherwise careful. + * + * WHY THE ASSERTIONS ARE BEHAVIOURAL. `expect(plugin.destroy).toBeDefined()` + * would pass on a plugin the kernel still never reaches — the hook merely + * EXISTING is not the property that was missing, being CALLED BY THE KERNEL + * is. So these drive a real `LiteKernel` through a real bootstrap and a real + * shutdown, and read a real `FileSystemRepository` handle. + * + * EVERY PRE-SHUTDOWN LEG IS A POSITIVE CONTROL and load-bearing: without it a + * plugin that never attached a repository would satisfy the post-shutdown + * assertion vacuously. + * + * THE `stop()` LEG IS THE OTHER DIRECTION, and it is not decoration: the + * repair keeps `stop()` as a delegating alias because it is public API of an + * exported class and an embedder may have learned to call it directly + * PRECISELY BECAUSE the kernel never did. Pinning only the shutdown direction + * would go green on an implementation that simply deletes `stop()`. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { LiteKernel } from '@objectstack/core'; +import type { PluginContext } from '@objectstack/core'; +import { MetadataPlugin } from './plugin.js'; + +const temps: string[] = []; + +function tempRoot(): string { + const dir = mkdtempSync(join(tmpdir(), 'metadata-plugin-teardown-')); + temps.push(dir); + return dir; +} + +/** Boot a real kernel carrying a real MetadataPlugin over a scratch root. */ +async function boot() { + const rootDir = tempRoot(); + const kernel = new LiteKernel(); + const plugin = new MetadataPlugin({ rootDir, watch: false }); + kernel.use(plugin); + await kernel.bootstrap(); + + const manager = kernel.getService<{ + getRepository(): { close(): Promise } | undefined; + }>('metadata'); + + return { kernel, plugin, manager, rootDir }; +} + +/** + * Count `close()` calls on the REAL repository handle the plugin attached — + * the same object the plugin holds, since `start()` assigns one instance to + * both itself and the manager. The real close still runs. + */ +function countCloses(repo: { close(): Promise }): () => number { + let closes = 0; + const real = repo.close.bind(repo); + repo.close = async () => { closes += 1; await real(); }; + return () => closes; +} + +afterEach(() => { + while (temps.length) rmSync(temps.pop()!, { recursive: true, force: true }); +}); + +describe('#10772 MetadataPlugin releases its repository on kernel shutdown', () => { + it('closes the metadata repository once shutdown() has resolved', async () => { + const { kernel, manager } = await boot(); + + // POSITIVE CONTROL — a repository really was attached, so the + // assertion below measures a release and not an absence. + const repo = manager.getRepository(); + expect(repo).toBeDefined(); + const closes = countCloses(repo!); + expect(closes()).toBe(0); + + await kernel.shutdown(); + + // THE PIN. Before the fix this stayed 0: the kernel had no `destroy()` + // to call, and `stop()` was never anybody's business. + expect(closes()).toBe(1); + }); + + it('the kernel reaches destroy() during shutdown', async () => { + const { kernel, plugin } = await boot(); + + let reached = 0; + const real = plugin.destroy; + plugin.destroy = async () => { reached += 1; await real(); }; + + // POSITIVE CONTROL — bootstrap alone must not tear the plugin down. + expect(reached).toBe(0); + + await kernel.shutdown(); + + expect(reached).toBe(1); + }); + + it('the retained stop() alias still tears down for an embedder that calls it directly', async () => { + const { manager, plugin } = await boot(); + + const repo = manager.getRepository(); + expect(repo).toBeDefined(); + const closes = countCloses(repo!); + + // No argument — the shape an embedder writes against a property whose + // parameter the repair made optional. + await plugin.stop(); + + expect(closes()).toBe(1); + }); + + it('the stop() alias still accepts the PluginContext argument it used to require', async () => { + const { manager, plugin } = await boot(); + + const repo = manager.getRepository(); + const closes = countCloses(repo!); + + // The pre-repair signature was `stop(ctx: PluginContext)`, required. + // An embedder holding that call shape must keep compiling AND keep + // working — the entire reason the alias was retained. + const ctx = { + logger: { info() {}, warn() {}, error() {}, debug() {} }, + } as unknown as PluginContext; + await plugin.stop(ctx); + + expect(closes()).toBe(1); + }); + + it('the alias survives being detached from the instance', async () => { + // It is an arrow PROPERTY, not a method — `const { stop } = plugin` + // is a call shape the pre-repair class supported, so the repair must + // not quietly convert it into an unbound method. + const { manager, plugin } = await boot(); + + const repo = manager.getRepository(); + const closes = countCloses(repo!); + + const { stop } = plugin; + await stop(); + + expect(closes()).toBe(1); + }); + + it('a teardown on a plugin the kernel never started is a no-op rather than a throw', async () => { + // Idempotence matters because `destroy()` clears the handles it + // released; a teardown that only works once fails inside a suite, and + // the kernel calls it on every plugin it walks. + const plugin = new MetadataPlugin({ rootDir: tempRoot(), watch: false }); + await expect(plugin.destroy()).resolves.toBeUndefined(); + await expect(plugin.destroy()).resolves.toBeUndefined(); + await expect(plugin.stop()).resolves.toBeUndefined(); + }); +}); diff --git a/packages/metadata/src/plugin.ts b/packages/metadata/src/plugin.ts index 2dc31f9e78..4c9165674e 100644 --- a/packages/metadata/src/plugin.ts +++ b/packages/metadata/src/plugin.ts @@ -235,6 +235,13 @@ export class MetadataPlugin implements Plugin { private repository?: import('@objectstack/metadata-core').MetadataRepository; /** Chokidar watcher on the artifact file (local-file mode) — ADR-0008 PR-8. */ private artifactWatcher?: { close: () => Promise }; + /** + * The context handed to `init()`, retained so `destroy()` can log. + * [#10772] `Plugin.destroy()` takes NO argument — it is the kernel's only + * teardown hook — so the context the old `stop(ctx)` alias received has to + * be captured at init time instead of arriving at teardown time. + */ + private initCtx?: PluginContext; /** * The most recently parsed artifact metadata (the plural-field record: * `objects`, `views`, `data`, …). Carried on the `metadata:reloaded` @@ -280,6 +287,9 @@ export class MetadataPlugin implements Plugin { } init = async (ctx: PluginContext) => { + // [#10772] Retained for `destroy()`, which the kernel calls with no + // context. Assigned before anything that can throw. + this.initCtx = ctx; ctx.logger.info('Initializing Metadata Manager', { root: this.options.rootDir || process.cwd(), watch: this.options.watch, @@ -543,7 +553,22 @@ export class MetadataPlugin implements Plugin { } } - stop = async (ctx: PluginContext) => { + /** + * Teardown — the kernel's ONLY teardown hook. + * + * [#10772] This body used to be spelled `stop(ctx)`. `Plugin` + * (`@objectstack/core`'s `types.ts`) declares `init()`, `start?(ctx)` and + * `destroy?()` and no `stop()`, and `ObjectKernel.performShutdown()` / + * `LiteKernel.destroy()` walk the plugins in reverse calling + * `plugin.destroy()` — so the artifact watcher, the manager and the + * repository were all still held after `await kernel.shutdown()` had + * RESOLVED. The `start`/`stop` pair read symmetric to a reviewer because + * `start()` really is on the interface; only one half was ever called. + * + * Idempotent: every handle is cleared as it is released, so a second + * teardown is a no-op rather than a second close. + */ + destroy = async (): Promise => { if (this.artifactWatcher) { try { await this.artifactWatcher.close(); } catch { /* noop */ } this.artifactWatcher = undefined; @@ -551,7 +576,7 @@ export class MetadataPlugin implements Plugin { try { await this.manager.dispose(); } catch (e: any) { - ctx.logger.warn('[MetadataPlugin] manager.dispose() failed', { error: e?.message }); + this.initCtx?.logger?.warn?.('[MetadataPlugin] manager.dispose() failed', { error: e?.message }); } const repo = this.repository as any; if (repo && typeof repo.close === 'function') { @@ -560,6 +585,19 @@ export class MetadataPlugin implements Plugin { this.repository = undefined; } + /** + * Retained alias for {@link destroy}. Kept because it is public API of an + * exported class: an embedder may have learned to call it directly + * precisely BECAUSE the kernel never did, and deleting it would break them. + * Still an arrow property, so a detached `const { stop } = plugin` call + * keeps working too. The parameter is now optional and ignored — + * `destroy()` takes no context, so teardown logs through the context + * captured in `init()`. + */ + stop = async (_ctx?: PluginContext): Promise => { + await this.destroy(); + } + /** * Fetch JSON content from a URL with configurable timeout. */ diff --git a/scripts/check-plugin-teardown-shape.mjs b/scripts/check-plugin-teardown-shape.mjs index 3179730fbb..3df871e10b 100644 --- a/scripts/check-plugin-teardown-shape.mjs +++ b/scripts/check-plugin-teardown-shape.mjs @@ -233,7 +233,6 @@ const POSITIVE_CONTROL = { * `EmailServicePlugin` / `WebhookOutboxPlugin` spell it `dispose`. */ const KNOWN_TEARDOWN_UNREACHED = [ - { file: 'packages/metadata/src/plugin.ts', cls: 'MetadataPlugin', alias: 'stop', repair: '#10371' }, { file: 'packages/plugins/plugin-email/src/email-plugin.ts', cls: 'EmailServicePlugin', alias: 'dispose', repair: '#10371' }, { file: 'packages/plugins/plugin-webhooks/src/webhook-outbox-plugin.ts', cls: 'WebhookOutboxPlugin', alias: 'dispose', repair: '#10371' }, { file: 'packages/runtime/src/app-plugin.ts', cls: 'AppPlugin', alias: 'stop', repair: '#10371' }, From 931d5e0a62d4cf76ce0b23093556bb3a2dc3c30c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 14:19:39 +0000 Subject: [PATCH 2/7] fix(runtime): AppPlugin emits app:unregistered from destroy(), not stop() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [#10772] `AppPlugin` emits `app:registered` on the kernel bus at start so the control plane's `AppCatalogService` can mirror the app into `sys_app`, and it emitted the matching `app:unregistered` from a teardown spelled `stop = async (ctx) => …`. `Plugin` (`@objectstack/core`'s `types.ts`) declares `init()`, `start?(ctx)` and `destroy?()` — and no `stop()` — and `ObjectKernel.performShutdown()` / `LiteKernel.destroy()` walk the plugins in reverse calling `plugin.destroy()`. Nothing in the repo calls `stop()` on a plugin, so `app:unregistered` was never emitted on a real shutdown and the catalog row outlived the kernel that registered it. The alias is an arrow PROPERTY, which is why a method-only reading of the class missed it. Repair is the PR-10375 shape: the body moves into `destroy()` and `stop()` stays as a delegating arrow property, so both a direct call and a detached `const { stop } = plugin` keep working. `destroy()` takes no context, so `init()` now captures the one it needs — assigned as init's first statement, ahead of the empty-env early return, so teardown is armed on every path init takes. Baseline: the `AppPlugin` line is deleted from `KNOWN_TEARDOWN_UNREACHED` in `scripts/check-plugin-teardown-shape.mjs`, on that gate's own stale-entry verdict rather than by line number. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx --- ...plugin-shutdown-emits-unregistered.test.ts | 166 ++++++++++++++++++ packages/runtime/src/app-plugin.ts | 45 ++++- scripts/check-plugin-teardown-shape.mjs | 1 - 3 files changed, 210 insertions(+), 2 deletions(-) create mode 100644 packages/runtime/src/app-plugin-shutdown-emits-unregistered.test.ts diff --git a/packages/runtime/src/app-plugin-shutdown-emits-unregistered.test.ts b/packages/runtime/src/app-plugin-shutdown-emits-unregistered.test.ts new file mode 100644 index 0000000000..c8aa540ceb --- /dev/null +++ b/packages/runtime/src/app-plugin-shutdown-emits-unregistered.test.ts @@ -0,0 +1,166 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#10772] `await kernel.shutdown()` must actually reach `AppPlugin`'s + * teardown. + * + * THE DEFECT. `AppPlugin` emits `app:registered` on the kernel bus at start so + * the control plane's `AppCatalogService` can upsert the `sys_app` row, and it + * emitted the matching `app:unregistered` from a teardown spelled + * `stop = async (ctx) => …`. `Plugin` (`@objectstack/core`'s `types.ts`) + * declares `init()`, `start?(ctx)` and `destroy?()` — and NO `stop()` — so + * `ObjectKernel.performShutdown()` and `LiteKernel.destroy()`, which walk the + * plugins in reverse calling `plugin.destroy()`, walked straight past it. + * Nothing in the repo ever called `stop()` on a plugin, so the catalog row + * outlived every kernel that registered it. + * + * WHY THE #10371 CENSUS MISSED IT. The alias is an arrow PROPERTY, not a + * method, so a method-only reading of the class does not see it at all. + * + * THE ASYMMETRY THAT HID IT. `start?(ctx)` IS on the interface and does fire. + * A `start`/`stop` pair where only one half is wired reads as symmetric in + * review — which is the whole reason this shape survived in eleven classes. + * + * WHY THE ASSERTIONS ARE BEHAVIOURAL. `expect(plugin.destroy).toBeDefined()` + * would pass on a plugin the kernel still never reaches. These drive a real + * `LiteKernel` through a real bootstrap and a real shutdown and read the + * events that actually landed on the bus. + * + * EVERY PRE-SHUTDOWN LEG IS A POSITIVE CONTROL: without it a plugin that never + * registered anything would satisfy the post-shutdown assertion vacuously. + * + * THE `stop()` LEG IS THE OTHER DIRECTION. The repair keeps `stop()` as a + * delegating alias because it is public API of an exported class and an + * embedder may have learned to call it directly PRECISELY BECAUSE the kernel + * never did. Pinning only the shutdown direction would go green on an + * implementation that simply deletes `stop()`. + */ + +import { describe, it, expect } from 'vitest'; +import { LiteKernel } from '@objectstack/core'; +import type { Plugin, PluginContext } from '@objectstack/core'; +import { ObjectQLPlugin } from '@objectstack/objectql'; +import { AppPlugin, type AppPluginProjectContext } from './app-plugin.js'; + +const PROJECT: AppPluginProjectContext = { + environmentId: 'env_1', + organizationId: 'org_1', + projectName: 'catalog-teardown', +}; + +const BUNDLE = { manifest: { id: 'demo_app', name: 'demo_app', label: 'Demo' } }; + +/** Captures the catalog events AppPlugin puts on the kernel bus. */ +class CatalogRecorderPlugin implements Plugin { + name = 'test.catalog-recorder'; + type = 'standard'; + version = '1.0.0'; + readonly events: string[] = []; + init(ctx: PluginContext): void { + ctx.hook('app:registered', () => { this.events.push('app:registered'); }); + ctx.hook('app:unregistered', () => { this.events.push('app:unregistered'); }); + } +} + +/** `emitCatalogEvent` does not await `ctx.trigger`, so let the bus settle. */ +const settle = () => new Promise((resolve) => setTimeout(resolve, 0)); + +/** + * The REAL composition an app kernel runs: the engine (which also provides the + * `manifest` service `AppPlugin` declares it cannot degrade without) plus the + * app itself. Booting without the engine makes `start()` return before it + * reaches the `app:registered` emit — which would leave the positive control + * below unsatisfiable and the pin vacuous. + */ +async function boot() { + const kernel = new LiteKernel({ logger: { level: 'error' } }); + const recorder = new CatalogRecorderPlugin(); + kernel.use(new ObjectQLPlugin({})); + kernel.use(recorder); + const plugin = new AppPlugin(BUNDLE, PROJECT, { skipSeedData: true }); + kernel.use(plugin); + await kernel.bootstrap(); + await settle(); + return { kernel, plugin, recorder }; +} + +describe('#10772 AppPlugin emits app:unregistered on kernel shutdown', () => { + it('puts app:unregistered on the bus once shutdown() has resolved', async () => { + const { kernel, recorder } = await boot(); + + // POSITIVE CONTROL — the catalog wiring really is live, so the + // assertion below measures an emit and not a dead hook. + expect(recorder.events).toContain('app:registered'); + expect(recorder.events).not.toContain('app:unregistered'); + + await kernel.shutdown(); + await settle(); + + // THE PIN. Before the fix this never arrived: the kernel had no + // `destroy()` to call, and `stop()` was never anybody's business. + expect(recorder.events).toContain('app:unregistered'); + }); + + it('the kernel reaches destroy() during shutdown', async () => { + const { kernel, plugin } = await boot(); + + let reached = 0; + const real = plugin.destroy; + plugin.destroy = async () => { reached += 1; await real(); }; + + expect(reached).toBe(0); + + await kernel.shutdown(); + + expect(reached).toBe(1); + }); + + it('the retained stop() alias still tears down for an embedder that calls it directly', async () => { + const { plugin, recorder } = await boot(); + + expect(recorder.events).not.toContain('app:unregistered'); + + // No argument — the shape an embedder writes against a property whose + // parameter the repair made optional. + await plugin.stop(); + await settle(); + + expect(recorder.events).toContain('app:unregistered'); + }); + + it('the stop() alias still accepts the PluginContext argument it used to require', async () => { + const { plugin, recorder } = await boot(); + + // The pre-repair signature was `stop(ctx: PluginContext)`, required. + // An embedder holding that call shape must keep compiling AND keep + // working — the entire reason the alias was retained. + const ctx = { + logger: { info() {}, warn() {}, error() {}, debug() {} }, + } as unknown as PluginContext; + await plugin.stop(ctx); + await settle(); + + expect(recorder.events).toContain('app:unregistered'); + }); + + it('the alias survives being detached from the instance', async () => { + // It is an arrow PROPERTY, not a method — `const { stop } = plugin` is + // a call shape the pre-repair class supported, so the repair must not + // quietly convert it into an unbound method. + const { plugin, recorder } = await boot(); + + const { stop } = plugin; + await stop(); + await settle(); + + expect(recorder.events).toContain('app:unregistered'); + }); + + it('a teardown on a plugin the kernel never initialized is a no-op rather than a throw', async () => { + // The kernel calls `destroy()` on every plugin it walks, including one + // whose `init()` never ran because an earlier plugin threw. + const plugin = new AppPlugin(BUNDLE, PROJECT, { skipSeedData: true }); + await expect(plugin.destroy()).resolves.toBeUndefined(); + await expect(plugin.stop()).resolves.toBeUndefined(); + }); +}); diff --git a/packages/runtime/src/app-plugin.ts b/packages/runtime/src/app-plugin.ts index 21cda81ea4..62c9cfb99b 100644 --- a/packages/runtime/src/app-plugin.ts +++ b/packages/runtime/src/app-plugin.ts @@ -87,6 +87,14 @@ export class AppPlugin implements Plugin { private bundle: any; private projectContext?: AppPluginProjectContext; + /** + * The context handed to `init()`, retained so `destroy()` can emit the + * `app:unregistered` catalog event. [#10772] `Plugin.destroy()` takes NO + * argument — it is the kernel's only teardown hook — so the context the + * old `stop(ctx)` alias received has to be captured at init time instead + * of arriving at teardown time. + */ + private initCtx?: PluginContext; /** When true, init/start become no-ops — env has no app payload. */ private readonly empty: boolean = false; /** @@ -176,6 +184,11 @@ export class AppPlugin implements Plugin { } init = async (ctx: PluginContext) => { + // [#10772] Retained for `destroy()`, which the kernel calls with no + // context. Assigned before anything that can throw, and before the + // empty-env early return, so teardown is armed on every path init + // takes. + this.initCtx = ctx; // Install the engine-wide default hook body runner FIRST — even for // empty envs (an empty env is exactly where a user will author their // first Studio hook). Runs in init (Phase 1) so it is in place before @@ -1419,11 +1432,41 @@ export class AppPlugin implements Plugin { }); } - stop = async (ctx: PluginContext) => { + /** + * Teardown — the kernel's ONLY teardown hook. + * + * [#10772] This body used to be spelled `stop(ctx)`. `Plugin` + * (`@objectstack/core`'s `types.ts`) declares `init()`, `start?(ctx)` and + * `destroy?()` and no `stop()`, and `ObjectKernel.performShutdown()` / + * `LiteKernel.destroy()` walk the plugins in reverse calling + * `plugin.destroy()` — so `app:unregistered` was never emitted on a real + * shutdown and the control plane's `sys_app` row outlived the kernel that + * registered it. The `start`/`stop` pair read symmetric to a reviewer + * because `start()` really is on the interface; only one half was called. + * + * No-ops without a project context or a captured context, exactly as the + * alias did. + */ + destroy = async (): Promise => { + const ctx = this.initCtx; + if (!ctx) return; const sys = this.bundle.manifest || this.bundle; this.emitCatalogEvent(ctx, 'app:unregistered', sys); } + /** + * Retained alias for {@link destroy}. Kept because it is public API of an + * exported class: an embedder may have learned to call it directly + * precisely BECAUSE the kernel never did, and deleting it would break them. + * Still an arrow property, so a detached `const { stop } = plugin` call + * keeps working too. The parameter is now optional and ignored — + * `destroy()` takes no context, so teardown uses the context captured in + * `init()`. + */ + stop = async (_ctx?: PluginContext): Promise => { + await this.destroy(); + } + /** * Emit a kernel hook so the control-plane `AppCatalogService` can * upsert / delete the corresponding `sys_app` row. Silently no-ops diff --git a/scripts/check-plugin-teardown-shape.mjs b/scripts/check-plugin-teardown-shape.mjs index 3df871e10b..0b3c6c0af8 100644 --- a/scripts/check-plugin-teardown-shape.mjs +++ b/scripts/check-plugin-teardown-shape.mjs @@ -235,7 +235,6 @@ const POSITIVE_CONTROL = { const KNOWN_TEARDOWN_UNREACHED = [ { file: 'packages/plugins/plugin-email/src/email-plugin.ts', cls: 'EmailServicePlugin', alias: 'dispose', repair: '#10371' }, { file: 'packages/plugins/plugin-webhooks/src/webhook-outbox-plugin.ts', cls: 'WebhookOutboxPlugin', alias: 'dispose', repair: '#10371' }, - { file: 'packages/runtime/src/app-plugin.ts', cls: 'AppPlugin', alias: 'stop', repair: '#10371' }, { file: 'packages/runtime/src/external-validation-plugin.ts', cls: 'ExternalValidationPlugin', alias: 'stop', repair: '#10371' }, ]; From f270c1569bf232acf6ac88da1892804a2f70c03d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 14:22:19 +0000 Subject: [PATCH 3/7] fix(runtime): ExternalValidationPlugin clears drift timers from destroy(), not stop() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [#10772] This is the load-bearing member of the family. The plugin arms one `setInterval` per opted-in datasource from `scheduleDriftChecks()` at `kernel:ready` (ADR-0015 §5.2), and the `clearInterval` sweep over them was spelled `stop = (): void => …`. `Plugin` (`@objectstack/core`'s `types.ts`) declares `init()`, `start?(ctx)` and `destroy?()` — and no `stop()` — and `ObjectKernel.performShutdown()` / `LiteKernel.destroy()` walk the plugins in reverse calling `plugin.destroy()`. The only caller `stop()` had anywhere in the tree was this class's own `scheduleDriftChecks()` re-arming itself, so on kernel shutdown those intervals were never cleared: the #9371 mechanism verbatim, in one of only two `Plugin` implementations in this tree that own `setInterval` at all (`ReportsServicePlugin` is the other, repaired under #10371). The plugin is mounted on the real serve path (`kernel.use(createExternalValidationPlugin())` in `packages/cli/src/commands/serve.ts`), so the leak is not confined to tests; the timers are `unref`'d, which is why it stayed silent in a long-lived host and lands in a vitest worker instead. Repair is the PR-10375 shape: the body moves into `destroy()` and `stop()` stays as a delegating alias. It stays a SYNCHRONOUS arrow property returning `void` — widening a public alias to `Promise` would change what an embedder's non-awaiting call site does. `scheduleDriftChecks()` now re-arms through `destroy()`, the canonical hook, leaving the alias purely for embedders. Baseline: the `ExternalValidationPlugin` line is deleted from `KNOWN_TEARDOWN_UNREACHED` in `scripts/check-plugin-teardown-shape.mjs`, on that gate's own stale-entry verdict rather than by line number. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx --- .../runtime/src/external-validation-plugin.ts | 43 ++++- ...-validation-shutdown-clears-timers.test.ts | 167 ++++++++++++++++++ scripts/check-plugin-teardown-shape.mjs | 1 - 3 files changed, 207 insertions(+), 4 deletions(-) create mode 100644 packages/runtime/src/external-validation-shutdown-clears-timers.test.ts diff --git a/packages/runtime/src/external-validation-plugin.ts b/packages/runtime/src/external-validation-plugin.ts index 4eb27aaa43..9c370df26d 100644 --- a/packages/runtime/src/external-validation-plugin.ts +++ b/packages/runtime/src/external-validation-plugin.ts @@ -178,12 +178,47 @@ export class ExternalValidationPlugin implements Plugin { }); }; - /** Tear down background drift-check timers (idempotent). */ - stop = (): void => { + /** + * Tear down background drift-check timers (idempotent) — the kernel's ONLY + * teardown hook. + * + * [#10772] This body used to be spelled `stop()`. `Plugin` + * (`@objectstack/core`'s `types.ts`) declares `init()`, `start?(ctx)` and + * `destroy?()` and no `stop()`, and `ObjectKernel.performShutdown()` / + * `LiteKernel.destroy()` walk the plugins in reverse calling + * `plugin.destroy()`. Nothing in the tree ever called `stop()` on a plugin, + * and this class's own only caller was `scheduleDriftChecks()` re-arming + * itself — so every armed `setInterval` below was STILL ARMED after + * `await kernel.shutdown()` had RESOLVED. That is #9371's mechanism + * verbatim, and this plugin is one of only two `Plugin` implementations in + * the tree that own `setInterval` at all (`ReportsServicePlugin` is the + * other, repaired under #10371). + * + * The timers are `unref`'d, so a long-lived host process still exits and + * nothing complains in production — the bill lands in a vitest worker, which + * is alive throughout teardown. + * + * Stays SYNCHRONOUS on purpose: `stop()` was `(): void`, and widening a + * public alias to `Promise` would change what an embedder's + * non-awaiting call site does. + */ + destroy = (): void => { for (const timer of this.driftTimers.values()) clearInterval(timer); this.driftTimers.clear(); }; + /** + * Retained alias for {@link destroy}. Kept because it is public API of an + * exported class: an embedder may have learned to call it directly precisely + * BECAUSE the kernel never did, and deleting it would break them. Still an + * arrow property returning `void`, so both a detached + * `const { stop } = plugin` call and a non-awaiting call site keep working + * unchanged. + */ + stop = (): void => { + this.destroy(); + }; + /** Exposed for testing; invoked from the kernel:ready handler. */ async runValidation(ctx: PluginContext): Promise { const svc = safeGet(ctx, 'external-datasource'); @@ -237,7 +272,9 @@ export class ExternalValidationPlugin implements Plugin { * don't accumulate. */ async scheduleDriftChecks(ctx: PluginContext): Promise { - this.stop(); + // [#10772] The canonical hook, not the retained alias: `destroy()` is now + // where the body lives, and the alias exists only for embedders. + this.destroy(); const metadata = safeGet(ctx, 'metadata'); if (!metadata?.list) return; diff --git a/packages/runtime/src/external-validation-shutdown-clears-timers.test.ts b/packages/runtime/src/external-validation-shutdown-clears-timers.test.ts new file mode 100644 index 0000000000..2d98b8f317 --- /dev/null +++ b/packages/runtime/src/external-validation-shutdown-clears-timers.test.ts @@ -0,0 +1,167 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#10772] `await kernel.shutdown()` must actually clear this plugin's armed + * drift-check intervals. + * + * THE DEFECT, AND WHY THIS MEMBER IS THE LOAD-BEARING ONE. + * `ExternalValidationPlugin` arms one `setInterval` per opted-in datasource + * from `scheduleDriftChecks()` at `kernel:ready` (ADR-0015 §5.2), keyed by + * name in `driftTimers`. The `clearInterval` sweep over them was spelled + * `stop = (): void => …`. `Plugin` (`@objectstack/core`'s `types.ts`) declares + * `init()`, `start?(ctx)` and `destroy?()` — and NO `stop()` — so + * `ObjectKernel.performShutdown()` and `LiteKernel.destroy()`, which walk the + * plugins in reverse calling `plugin.destroy()`, walked straight past it. The + * only caller `stop()` had anywhere in the tree was this class's own + * `scheduleDriftChecks()` re-arming itself. So on kernel shutdown the + * intervals were NEVER cleared — the #9371 mechanism verbatim, in one of only + * two `Plugin` implementations in this tree that own `setInterval` at all + * (`ReportsServicePlugin` is the other, repaired under #10371). + * + * This plugin is mounted on the real serve path + * (`packages/cli/src/commands/serve.ts` — `kernel.use(createExternalValidationPlugin())`), + * so the leak is not confined to tests. + * + * WHY THE LEAK STAYED SILENT. `scheduleDriftChecks()` `unref()`s each timer, + * so a long-lived host process still exits and nothing complains in + * production. Under vitest the worker is alive throughout teardown, so a tick + * fires after the file is over and lands in whatever the suite has already + * disconnected — which is exactly how #9371's bill arrived, as merge-queue + * evictions of runs in which every test passed. + * + * WHY THE ASSERTIONS ARE BEHAVIOURAL. `expect(plugin.destroy).toBeDefined()` + * would pass on a plugin the kernel still never reaches. These drive a real + * `LiteKernel` through a real bootstrap (so the timers are armed by the real + * `kernel:ready` path) and a real shutdown, and read the timer count and the + * drift checker's own call count. + * + * EVERY PRE-SHUTDOWN LEG IS A POSITIVE CONTROL: without it a plugin that armed + * nothing would satisfy the post-shutdown assertion vacuously. + * + * THE `stop()` LEG IS THE OTHER DIRECTION. The repair keeps `stop()` as a + * delegating alias because it is public API of an exported class and an + * embedder may have learned to call it directly PRECISELY BECAUSE the kernel + * never did. Pinning only the shutdown direction would go green on an + * implementation that simply deletes `stop()`. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { LiteKernel } from '@objectstack/core'; +import type { Plugin, PluginContext } from '@objectstack/core'; +import { ExternalValidationPlugin } from './external-validation-plugin.js'; + +const INTERVAL_MS = 1000; + +/** Counts the drift checker's reads, so "still ticking" is measurable. */ +class FakeFederationPlugin implements Plugin { + name = 'test.federation'; + type = 'standard'; + version = '1.0.0'; + providesServices = ['external-datasource', 'metadata']; + validateAllCalls = 0; + init(ctx: PluginContext): void { + ctx.registerService('external-datasource', { + validateAll: async () => { + this.validateAllCalls += 1; + return { ok: true, results: [] }; + }, + }); + ctx.registerService('metadata', { + list: async () => [ + { name: 'warehouse', external: { validation: { checkIntervalMs: INTERVAL_MS } } }, + ], + }); + } +} + +async function boot() { + const kernel = new LiteKernel({ logger: { level: 'error' } }); + const federation = new FakeFederationPlugin(); + kernel.use(federation); + const plugin = new ExternalValidationPlugin(); + kernel.use(plugin); + await kernel.bootstrap(); + return { kernel, plugin, federation }; +} + +beforeEach(() => { vi.useFakeTimers(); }); +afterEach(() => { vi.useRealTimers(); }); + +describe('#10772 ExternalValidationPlugin clears its drift timers on kernel shutdown', () => { + it('leaves no armed interval once shutdown() has resolved', async () => { + const { kernel } = await boot(); + + // POSITIVE CONTROL — a drift timer really is armed by the real + // `kernel:ready` path, so the assertion below measures a release. + expect(vi.getTimerCount()).toBe(1); + + await kernel.shutdown(); + + // THE PIN. Before the fix this stayed 1: the kernel had no `destroy()` + // to call, and `stop()`'s only caller was this class re-arming itself. + expect(vi.getTimerCount()).toBe(0); + }); + + it('issues no further drift reads once shutdown() has resolved', async () => { + const { kernel, federation } = await boot(); + + // POSITIVE CONTROL — the armed timer really does fire and really does + // read, so a count that stops climbing means something. + await vi.advanceTimersByTimeAsync(INTERVAL_MS); + expect(federation.validateAllCalls).toBeGreaterThan(0); + + await kernel.shutdown(); + const atShutdown = federation.validateAllCalls; + + await vi.advanceTimersByTimeAsync(INTERVAL_MS * 5); + + // THE PIN. shutdown() resolving means the plugin is done reading. + // Before the fix this count kept climbing. + expect(federation.validateAllCalls).toBe(atShutdown); + }); + + it('the kernel reaches destroy() during shutdown', async () => { + const { kernel, plugin } = await boot(); + + let reached = 0; + const real = plugin.destroy; + plugin.destroy = () => { reached += 1; real(); }; + + expect(reached).toBe(0); + + await kernel.shutdown(); + + expect(reached).toBe(1); + }); + + it('the retained stop() alias still clears the timers for a direct caller', async () => { + const { plugin } = await boot(); + + expect(vi.getTimerCount()).toBe(1); + + plugin.stop(); + + expect(vi.getTimerCount()).toBe(0); + }); + + it('the alias stays SYNCHRONOUS and survives being detached from the instance', async () => { + // It was `stop = (): void =>`, an arrow property. A non-awaiting call + // site and a detached `const { stop } = plugin` are both call shapes + // the pre-repair class supported; widening the alias to a Promise, or + // converting it to an unbound method, would break them. + const { plugin } = await boot(); + + const { stop } = plugin; + const returned = stop(); + + expect(returned).toBeUndefined(); + expect(vi.getTimerCount()).toBe(0); + }); + + it('a teardown on a plugin that armed nothing is a no-op rather than a throw', async () => { + const plugin = new ExternalValidationPlugin(); + expect(() => plugin.destroy()).not.toThrow(); + expect(() => plugin.destroy()).not.toThrow(); + expect(() => plugin.stop()).not.toThrow(); + }); +}); diff --git a/scripts/check-plugin-teardown-shape.mjs b/scripts/check-plugin-teardown-shape.mjs index 0b3c6c0af8..6caf06f657 100644 --- a/scripts/check-plugin-teardown-shape.mjs +++ b/scripts/check-plugin-teardown-shape.mjs @@ -235,7 +235,6 @@ const POSITIVE_CONTROL = { const KNOWN_TEARDOWN_UNREACHED = [ { file: 'packages/plugins/plugin-email/src/email-plugin.ts', cls: 'EmailServicePlugin', alias: 'dispose', repair: '#10371' }, { file: 'packages/plugins/plugin-webhooks/src/webhook-outbox-plugin.ts', cls: 'WebhookOutboxPlugin', alias: 'dispose', repair: '#10371' }, - { file: 'packages/runtime/src/external-validation-plugin.ts', cls: 'ExternalValidationPlugin', alias: 'stop', repair: '#10371' }, ]; // --------------------------------------------------------------------------- From b3e53e70f68cbcc577a335d86263444904b4e427 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 14:26:24 +0000 Subject: [PATCH 4/7] fix(plugin-email): EmailServicePlugin tears down from destroy(), not dispose() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [#10772] The plugin arms a live `email_template` bridge at `kernel:ready`: a metadata subscription, a protocol mutation listener, a provenance hook bound to the data engine, and — when SMTP is configured — an open transport. The teardown that released all of them was spelled `dispose()`. `Plugin` (`@objectstack/core`'s `types.ts`) declares `init()`, `start?(ctx)` and `destroy?()` — and no `dispose()` — and `ObjectKernel.performShutdown()` / `LiteKernel.destroy()` walk the plugins in reverse calling `plugin.destroy()`. Measured repo-wide on this revision: `dispose()` had exactly ONE caller anywhere, a test in this package. The kernel was never one of them, so after `await kernel.shutdown()` had resolved the bridge was still armed and still writing `sys_email_template` rows through an engine the suite had moved on from. This member and `WebhookOutboxPlugin` are the `dispose()` half of the family — the seventh spelling the #10619 gate's roster was widened for before any instance of it was known, already present when the roster was measured. A census looking only for `stop()` misses both. Repair is the PR-10375 shape: the body moves into `destroy()` and `dispose()` stays as a delegating alias with the same signature and return type — public API of an exported class, and it has a live direct caller in this package. Baseline: the `EmailServicePlugin` line is deleted from `KNOWN_TEARDOWN_UNREACHED` in `scripts/check-plugin-teardown-shape.mjs`, on that gate's own stale-entry verdict rather than by line number. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx --- .../plugins/plugin-email/src/email-plugin.ts | 28 ++- ...-shutdown-detaches-template-bridge.test.ts | 221 ++++++++++++++++++ scripts/check-plugin-teardown-shape.mjs | 1 - 3 files changed, 248 insertions(+), 2 deletions(-) create mode 100644 packages/plugins/plugin-email/src/plugin-shutdown-detaches-template-bridge.test.ts diff --git a/packages/plugins/plugin-email/src/email-plugin.ts b/packages/plugins/plugin-email/src/email-plugin.ts index 3f8308ba66..5773c297bf 100644 --- a/packages/plugins/plugin-email/src/email-plugin.ts +++ b/packages/plugins/plugin-email/src/email-plugin.ts @@ -1252,7 +1252,23 @@ export class EmailServicePlugin implements Plugin { return stripReadDecorations(item); } - async dispose(): Promise { + /** + * Teardown — the kernel's ONLY teardown hook. + * + * [#10772] This body used to be spelled `dispose()`. `Plugin` + * (`@objectstack/core`'s `types.ts`) declares `init()`, `start?(ctx)` and + * `destroy?()` and no `dispose()`, and `ObjectKernel.performShutdown()` / + * `LiteKernel.destroy()` walk the plugins in reverse calling + * `plugin.destroy()` — so after `await kernel.shutdown()` had RESOLVED, the + * two metadata subscriptions were still live, the SMTP transport was still + * open and the provenance hook was still bound to the engine. `dispose()` + * had exactly ONE caller in the whole repo, a test in this package; the + * kernel was never one of them. + * + * Idempotent: every handle is cleared as it is released, so a second + * teardown is a no-op rather than a second close. + */ + async destroy(): Promise { this.templateBridgeArmed = false; try { this.unsubscribeTemplates?.(); } catch { /* best effort */ } this.unsubscribeTemplates = undefined; @@ -1268,6 +1284,16 @@ export class EmailServicePlugin implements Plugin { } } + /** + * Retained alias for {@link destroy}. Kept because it is public API of an + * exported class: an embedder may have learned to call it directly precisely + * BECAUSE the kernel never did, and deleting it would break them. Same + * signature, same return type — a direct caller sees no change. + */ + async dispose(): Promise { + await this.destroy(); + } + /** * Translate the `mail` settings namespace snapshot into a transport * and `defaultFrom`, then hot-swap them on the running EmailService. diff --git a/packages/plugins/plugin-email/src/plugin-shutdown-detaches-template-bridge.test.ts b/packages/plugins/plugin-email/src/plugin-shutdown-detaches-template-bridge.test.ts new file mode 100644 index 0000000000..47d7ef5837 --- /dev/null +++ b/packages/plugins/plugin-email/src/plugin-shutdown-detaches-template-bridge.test.ts @@ -0,0 +1,221 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#10772] `await kernel.shutdown()` must actually reach `EmailServicePlugin`'s + * teardown. + * + * THE DEFECT. The plugin arms a live `email_template` bridge at + * `kernel:ready`: a metadata subscription, a protocol mutation listener, a + * provenance hook bound to the data engine, and (when SMTP is configured) an + * open transport. The teardown that released all of them was spelled + * `dispose()`. `Plugin` (`@objectstack/core`'s `types.ts`) declares `init()`, + * `start?(ctx)` and `destroy?()` — and NO `dispose()` — and + * `ObjectKernel.performShutdown()` / `LiteKernel.destroy()` walk the plugins + * in reverse calling `plugin.destroy()`. Measured on the same revision: + * `dispose()` had exactly ONE caller in the whole repo, a test in this + * package. The kernel was never one of them, so after `await + * kernel.shutdown()` had RESOLVED the bridge was still armed and still + * writing. + * + * THE SPELLING. This member and `WebhookOutboxPlugin` are the `dispose()` + * half of the family — the "seventh spelling" the #10619 gate's roster was + * widened for BEFORE any instance of it was known, already present when the + * roster was measured. A census that looked only for `stop()` misses them + * entirely, which is the other half of why #10371's enumeration came out + * short. + * + * WHY THE ASSERTIONS ARE BEHAVIOURAL. `expect(plugin.destroy).toBeDefined()` + * would pass on a plugin the kernel still never reaches. These drive a real + * `LiteKernel` through a real bootstrap and a real shutdown and then perform a + * real runtime write, reading the rows that did or did not land. + * + * EVERY PRE-SHUTDOWN LEG IS A POSITIVE CONTROL: without it a bridge that never + * armed would satisfy the post-shutdown assertion vacuously. + * + * THE `dispose()` LEG IS THE OTHER DIRECTION. The repair keeps `dispose()` as + * a delegating alias because it is public API of an exported class and an + * embedder — here, a test in this very package — calls it directly. Pinning + * only the shutdown direction would go green on an implementation that simply + * deletes `dispose()`. + */ + +import { describe, it, expect } from 'vitest'; +import { assertEngineUpdateDispatch } from '@objectstack/objectql'; +import { LiteKernel } from '@objectstack/core'; +import type { Plugin, PluginContext } from '@objectstack/core'; +import { EmailServicePlugin } from './email-plugin.js'; + +const TABLE = 'sys_email_template'; +type AnyRecord = Record; + +/** + * The slice of ObjectQL the template bridge and the provenance stamp touch — + * the same double `email-plugin.template-runtime-write.test.ts` uses, so this + * file cannot accept a dispatch shape the real engine would refuse. + */ +function fakeEngine() { + const rows: AnyRecord[] = []; + const matches = (row: AnyRecord, cond?: AnyRecord) => + !cond || + Object.entries(cond).every(([k, v]) => { + if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`); + return row[k] === v; + }); + return { + rows, + _registry: { listItems: () => [] }, + async find(object: string, q?: AnyRecord) { + if (object !== TABLE) return []; + const out = rows.filter((r) => matches(r, q?.where ?? q?.filter)); + return typeof q?.limit === 'number' ? out.slice(0, q.limit) : out; + }, + async insert(_object: string, row: AnyRecord) { + rows.push({ ...row }); + return { id: row.id }; + }, + async update(_object: string, data: AnyRecord, options?: AnyRecord) { + const dispatch = assertEngineUpdateDispatch(data, options); + if (dispatch.kind !== 'by-id') throw new Error(`unexpected update dispatch: ${dispatch.kind}`); + const target = rows.find((r) => r.id === dispatch.id); + if (target) Object.assign(target, data); + return { affected: target ? 1 : 0 }; + }, + registerHook() { /* provenance stamp */ }, + unregisterHooksByPackage() { return 0; }, + }; +} + +/** Mirrors `ObjectStackProtocolImplementation`'s two registration seams. */ +function fakeProtocol() { + const projectors = new Map Promise>(); + const listeners: Array<(evt: AnyRecord) => void> = []; + const p: AnyRecord = { + projectorFailures: [] as string[], + registerMutationProjector: (type: string, fn: (evt: AnyRecord) => Promise) => { + projectors.set(type, fn); + }, + onMetadataMutation: (fn: (evt: AnyRecord) => void) => { + listeners.push(fn); + return () => { + const i = listeners.indexOf(fn); + if (i >= 0) listeners.splice(i, 1); + }; + }, + async announce(evt: AnyRecord) { + const projector = projectors.get(evt.type); + if (projector) { + // Mirrors `runMutationProjector`: a throw is caught and + // reported on the write's response, never propagated. + try { await projector(evt); } + catch (e: any) { p.projectorFailures.push(String(e?.message ?? e)); } + } + for (const l of [...listeners]) l(evt); + await new Promise((r) => setTimeout(r, 0)); + }, + /** A `PUT /api/v1/meta/email_template/:name` that landed. */ + save: (name: string, body: unknown) => + p.announce({ type: 'email_template', name, state: 'active', body }), + }; + return p; +} + +const template = () => ({ + name: 'auth.password_reset', + label: 'Password Reset', + category: 'auth', + locale: 'en-US', + subject: 'Reset your password, {{user.name}}', + bodyHtml: '

Click here

', +}); + +/** Registers the collaborators the email plugin resolves. Nothing under test. */ +class FixturePlugin implements Plugin { + name = 'com.objectstack.engine.objectql'; + type = 'standard'; + version = '1.0.0'; + providesServices = ['objectql', 'manifest', 'metadata', 'protocol']; + readonly engine = fakeEngine(); + readonly protocol = fakeProtocol(); + init(ctx: PluginContext): void { + ctx.registerService('objectql', this.engine); + ctx.registerService('manifest', { register: () => {}, list: () => [] }); + ctx.registerService('metadata', { + list: () => [], + get: async () => undefined, + subscribe: () => () => {}, + }); + ctx.registerService('protocol', this.protocol); + } +} + +async function boot() { + const kernel = new LiteKernel({ logger: { level: 'error' } }); + const fixture = new FixturePlugin(); + kernel.use(fixture); + const plugin = new EmailServicePlugin({ seedTemplates: false }); + kernel.use(plugin); + await kernel.bootstrap(); + return { kernel, plugin, fixture }; +} + +const rowsOf = (engine: AnyRecord) => + engine.rows.filter((r: AnyRecord) => r.name === 'auth.password_reset'); + +describe('#10772 EmailServicePlugin detaches its template bridge on kernel shutdown', () => { + it('stops materializing runtime writes once shutdown() has resolved', async () => { + const { kernel, fixture } = await boot(); + + // POSITIVE CONTROL — the live bridge really is armed, so the assertion + // below measures a detachment and not a bridge that never worked. + await fixture.protocol.save('auth.password_reset', template()); + expect(fixture.protocol.projectorFailures).toEqual([]); + expect(rowsOf(fixture.engine)).toHaveLength(1); + fixture.engine.rows.length = 0; + + await kernel.shutdown(); + + await fixture.protocol.save('auth.password_reset', template()); + + // THE PIN. Before the fix this wrote another row: the kernel had no + // `destroy()` to call, and `dispose()`'s only caller in the entire + // repo was a test. + expect(rowsOf(fixture.engine)).toHaveLength(0); + }); + + it('the kernel reaches destroy() during shutdown', async () => { + const { kernel, plugin } = await boot(); + + let reached = 0; + const real = plugin.destroy.bind(plugin); + plugin.destroy = async () => { reached += 1; await real(); }; + + expect(reached).toBe(0); + + await kernel.shutdown(); + + expect(reached).toBe(1); + }); + + it('the retained dispose() alias still tears down for an embedder that calls it directly', async () => { + const { plugin, fixture } = await boot(); + + await fixture.protocol.save('auth.password_reset', template()); + expect(rowsOf(fixture.engine)).toHaveLength(1); + fixture.engine.rows.length = 0; + + await plugin.dispose(); + + await fixture.protocol.save('auth.password_reset', template()); + + expect(rowsOf(fixture.engine)).toHaveLength(0); + }); + + it('a teardown on a plugin the kernel never started is a no-op rather than a throw', async () => { + // Idempotence matters because `destroy()` clears the handles it + // released; the kernel calls it on every plugin it walks. + const plugin = new EmailServicePlugin({ seedTemplates: false }); + await expect(plugin.destroy()).resolves.toBeUndefined(); + await expect(plugin.destroy()).resolves.toBeUndefined(); + await expect(plugin.dispose()).resolves.toBeUndefined(); + }); +}); diff --git a/scripts/check-plugin-teardown-shape.mjs b/scripts/check-plugin-teardown-shape.mjs index 6caf06f657..37131210d2 100644 --- a/scripts/check-plugin-teardown-shape.mjs +++ b/scripts/check-plugin-teardown-shape.mjs @@ -233,7 +233,6 @@ const POSITIVE_CONTROL = { * `EmailServicePlugin` / `WebhookOutboxPlugin` spell it `dispose`. */ const KNOWN_TEARDOWN_UNREACHED = [ - { file: 'packages/plugins/plugin-email/src/email-plugin.ts', cls: 'EmailServicePlugin', alias: 'dispose', repair: '#10371' }, { file: 'packages/plugins/plugin-webhooks/src/webhook-outbox-plugin.ts', cls: 'WebhookOutboxPlugin', alias: 'dispose', repair: '#10371' }, ]; From cf70c3179c68fbf1ecfeb6b50aa8afe94e84acdc Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 14:31:18 +0000 Subject: [PATCH 5/7] fix(plugin-webhooks): WebhookOutboxPlugin tears down from destroy(), not dispose() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [#10772] At `kernel:ready` the plugin binds two hooks onto the data engine and starts an `AutoEnqueuer`, which holds two realtime subscriptions, a crypto-provider listener and a `setInterval` refresh timer. The teardown that released all of it was spelled `dispose()`. `Plugin` (`@objectstack/core`'s `types.ts`) declares `init()`, `start?(ctx)` and `destroy?()` — and no `dispose()` — and `ObjectKernel.performShutdown()` / `LiteKernel.destroy()` walk the plugins in reverse calling `plugin.destroy()`. Measured repo-wide on this revision: `dispose()` had ZERO callers anywhere — not the kernel, not a test, not an example — so this teardown had never run in any process at all. Repair is the PR-10375 shape: the body moves into `destroy()` and `dispose()` stays as a delegating alias with the same signature and return type — public API of an exported class, kept precisely because the zero-caller census is the argument for deleting it and the wrong one to act on. `vitest.config.ts` is new here and required rather than incidental: the pin needs a REAL kernel, which is this package's first VALUE import of `@objectstack/core` (the plugin's own `import type` is erased before resolution). `pnpm check:test-source-alias` reds on an unaliased value import resolving to `core/dist` and names this remedy — alias to source, do not widen the shrink-only `KNOWN_UNALIASED_TEST_IMPORTS`. The entry is an ANCHORED regex so it cannot swallow the published `@objectstack/core/logger` subpath, and `test` is left unset so the package keeps the vitest defaults its bare `vitest run` was relying on. Full suite after: 124/124. Baseline: the last entry — `WebhookOutboxPlugin` — is deleted from `KNOWN_TEARDOWN_UNREACHED`, on that gate's own stale-entry verdict rather than by line number, leaving the ratchet EMPTY. The header, the do-not-widen failure text and the verdict line are updated to match: the verdict now reads its repair card from the surviving entries' own `repair` data instead of hard-coding `#10371`, which is closed, and says "baseline fully burned down" when there are none. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx --- ...lugin-shutdown-stops-auto-enqueuer.test.ts | 178 ++++++++++++++++++ .../src/webhook-outbox-plugin.ts | 27 ++- .../plugins/plugin-webhooks/vitest.config.ts | 37 ++++ scripts/check-plugin-teardown-shape.mjs | 50 +++-- 4 files changed, 272 insertions(+), 20 deletions(-) create mode 100644 packages/plugins/plugin-webhooks/src/plugin-shutdown-stops-auto-enqueuer.test.ts create mode 100644 packages/plugins/plugin-webhooks/vitest.config.ts diff --git a/packages/plugins/plugin-webhooks/src/plugin-shutdown-stops-auto-enqueuer.test.ts b/packages/plugins/plugin-webhooks/src/plugin-shutdown-stops-auto-enqueuer.test.ts new file mode 100644 index 0000000000..7dfbd9bf23 --- /dev/null +++ b/packages/plugins/plugin-webhooks/src/plugin-shutdown-stops-auto-enqueuer.test.ts @@ -0,0 +1,178 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#10772] `await kernel.shutdown()` must actually reach `WebhookOutboxPlugin`'s + * teardown. + * + * THE DEFECT, AND HOW FAR IT WENT. At `kernel:ready` the plugin binds two + * hooks onto the data engine and starts an {@link AutoEnqueuer}, which holds + * TWO realtime subscriptions, a crypto-provider listener and a `setInterval` + * refresh timer. The teardown that released all of it was spelled `dispose()`. + * `Plugin` (`@objectstack/core`'s `types.ts`) declares `init()`, `start?(ctx)` + * and `destroy?()` — and NO `dispose()` — and `ObjectKernel.performShutdown()` + * / `LiteKernel.destroy()` walk the plugins in reverse calling + * `plugin.destroy()`. + * + * Measured repo-wide on this revision: `dispose()` had ZERO callers anywhere + * — not the kernel, not a test, not an example. This teardown had therefore + * never run in any process at all, and everything above outlived every kernel + * that started it. + * + * THE SPELLING. This member and `EmailServicePlugin` are the `dispose()` half + * of the family — the seventh spelling the #10619 gate's roster was widened + * for before any instance of it was known, already present when the roster was + * measured. A census looking only for `stop()` misses both. + * + * WHY THE ASSERTIONS ARE BEHAVIOURAL. `expect(plugin.destroy).toBeDefined()` + * would pass on a plugin the kernel still never reaches. These drive a real + * `LiteKernel` through a real bootstrap and a real shutdown and read the + * realtime service's own subscription ledger and the live timer count. + * + * EVERY PRE-SHUTDOWN LEG IS A POSITIVE CONTROL: without it a plugin that + * subscribed to nothing would satisfy the post-shutdown assertion vacuously. + * + * THE `dispose()` LEG IS THE OTHER DIRECTION. The repair keeps `dispose()` as + * a delegating alias because it is public API of an exported class, and an + * embedder may have learned to call it directly PRECISELY BECAUSE the kernel + * never did. Pinning only the shutdown direction would go green on an + * implementation that simply deletes `dispose()` — which, given the zero-caller + * census above, is exactly the shortcut this file exists to refuse. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { LiteKernel } from '@objectstack/core'; +import type { Plugin, PluginContext } from '@objectstack/core'; +import { WebhookOutboxPlugin } from './webhook-outbox-plugin.js'; + +type AnyRecord = Record; + +/** Realtime double whose whole job is to record what is still subscribed. */ +function fakeRealtime() { + let next = 0; + const live = new Set(); + return { + live, + async subscribe() { + const id = `sub_${++next}`; + live.add(id); + return id; + }, + async unsubscribe(id: string) { live.delete(id); }, + async publish() { /* nothing under test */ }, + }; +} + +/** The slice of ObjectQL the enqueuer's cache refresh and the hooks touch. */ +function fakeEngine() { + return { + registry: { listItems: () => [], getObject: () => undefined }, + async find() { return []; }, + async insert(_o: string, row: AnyRecord) { return { id: 'row_1', ...row }; }, + async update() { return { affected: 0 }; }, + async delete() { return true; }, + registerHook() { /* provenance stamp + headers gate */ }, + unregisterHooksByPackage() { return 0; }, + }; +} + +/** + * Stands in for the plugin's declared dependency + * (`dependencies = ['com.objectstack.service.messaging']`) and registers the + * collaborators it resolves. Nothing here is under test — the plugin's own + * teardown is. + */ +class FixturePlugin implements Plugin { + name = 'com.objectstack.service.messaging'; + type = 'standard'; + version = '1.0.0'; + providesServices = ['manifest', 'objectql', 'realtime', 'messaging']; + readonly realtime = fakeRealtime(); + readonly engine = fakeEngine(); + init(ctx: PluginContext): void { + ctx.registerService('manifest', { register: () => {}, list: () => [] }); + ctx.registerService('objectql', this.engine); + ctx.registerService('realtime', this.realtime); + ctx.registerService('messaging', { + enqueueHttp: async () => ({ id: 'delivery_1' }), + isHttpDeliveryReady: () => true, + registerRedeliverGuard: () => {}, + }); + } +} + +async function boot() { + const kernel = new LiteKernel({ logger: { level: 'error' } }); + const fixture = new FixturePlugin(); + kernel.use(fixture); + const plugin = new WebhookOutboxPlugin(); + kernel.use(plugin); + await kernel.bootstrap(); + return { kernel, plugin, fixture }; +} + +beforeEach(() => { vi.useFakeTimers(); }); +afterEach(() => { vi.useRealTimers(); }); + +describe('#10772 WebhookOutboxPlugin stops its auto-enqueuer on kernel shutdown', () => { + it('releases every realtime subscription once shutdown() has resolved', async () => { + const { kernel, fixture } = await boot(); + + // POSITIVE CONTROL — the enqueuer really did subscribe, so the + // assertion below measures a release and not an absence. + expect(fixture.realtime.live.size).toBe(2); + + await kernel.shutdown(); + + // THE PIN. Before the fix these stayed live: the kernel had no + // `destroy()` to call, and `dispose()` had no caller in the repo at + // all — this teardown had never run in any process. + expect(fixture.realtime.live.size).toBe(0); + }); + + it('leaves no armed refresh interval once shutdown() has resolved', async () => { + const { kernel } = await boot(); + + // POSITIVE CONTROL — the enqueuer's periodic refresh timer is armed. + // Note for the census: this plugin owns a `setInterval` TRANSITIVELY, + // through the collaborator it constructs, so a scan of the plugin + // class's own text does not see it. + expect(vi.getTimerCount()).toBeGreaterThan(0); + + await kernel.shutdown(); + + expect(vi.getTimerCount()).toBe(0); + }); + + it('the kernel reaches destroy() during shutdown', async () => { + const { kernel, plugin } = await boot(); + + let reached = 0; + const real = plugin.destroy.bind(plugin); + plugin.destroy = async () => { reached += 1; await real(); }; + + expect(reached).toBe(0); + + await kernel.shutdown(); + + expect(reached).toBe(1); + }); + + it('the retained dispose() alias still tears down for an embedder that calls it directly', async () => { + const { plugin, fixture } = await boot(); + + expect(fixture.realtime.live.size).toBe(2); + + await plugin.dispose(); + + expect(fixture.realtime.live.size).toBe(0); + }); + + it('a teardown on a plugin the kernel never started is a no-op rather than a throw', async () => { + // Idempotence matters because `destroy()` clears the handles it + // released; the kernel calls it on every plugin it walks. + const plugin = new WebhookOutboxPlugin(); + await expect(plugin.destroy()).resolves.toBeUndefined(); + await expect(plugin.destroy()).resolves.toBeUndefined(); + await expect(plugin.dispose()).resolves.toBeUndefined(); + }); +}); diff --git a/packages/plugins/plugin-webhooks/src/webhook-outbox-plugin.ts b/packages/plugins/plugin-webhooks/src/webhook-outbox-plugin.ts index 045683afcb..c52589c393 100644 --- a/packages/plugins/plugin-webhooks/src/webhook-outbox-plugin.ts +++ b/packages/plugins/plugin-webhooks/src/webhook-outbox-plugin.ts @@ -169,7 +169,22 @@ export class WebhookOutboxPlugin implements Plugin { }); } - async dispose(): Promise { + /** + * Teardown — the kernel's ONLY teardown hook. + * + * [#10772] This body used to be spelled `dispose()`. `Plugin` + * (`@objectstack/core`'s `types.ts`) declares `init()`, `start?(ctx)` and + * `destroy?()` and no `dispose()`, and `ObjectKernel.performShutdown()` / + * `LiteKernel.destroy()` walk the plugins in reverse calling + * `plugin.destroy()` — so after `await kernel.shutdown()` had RESOLVED the + * auto-enqueuer was still running and both engine hooks were still bound. + * Measured on the same revision: `dispose()` had ZERO callers anywhere in + * the repo, so this teardown had never run in any process at all. + * + * Idempotent: `boundEngine` is cleared as it is unbound, so a second + * teardown is a no-op rather than a second unbind. + */ + async destroy(): Promise { await this.autoEnqueuer?.stop(); if (this.boundEngine) { try { unbindWebhookProvenanceStamp(this.boundEngine); } catch { /* best effort */ } @@ -178,6 +193,16 @@ export class WebhookOutboxPlugin implements Plugin { } } + /** + * Retained alias for {@link destroy}. Kept because it is public API of an + * exported class: an embedder may have learned to call it directly + * precisely BECAUSE the kernel never did, and deleting it would break them. + * Same signature, same return type — a direct caller sees no change. + */ + async dispose(): Promise { + await this.destroy(); + } + private getMessaging(ctx: PluginContext): MessagingHttpSurface | undefined { const svc = this.tryGetService(ctx, ['messaging']); return svc && typeof svc.enqueueHttp === 'function' ? svc : undefined; diff --git a/packages/plugins/plugin-webhooks/vitest.config.ts b/packages/plugins/plugin-webhooks/vitest.config.ts new file mode 100644 index 0000000000..1851931a9f --- /dev/null +++ b/packages/plugins/plugin-webhooks/vitest.config.ts @@ -0,0 +1,37 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { defineConfig } from 'vitest/config'; +import path from 'node:path'; + +/** + * [#10772] This package had no vitest config until + * `plugin-shutdown-stops-auto-enqueuer.test.ts` needed a REAL kernel to prove + * that `await kernel.shutdown()` reaches `WebhookOutboxPlugin.destroy()`. That + * is the package's first VALUE import of `@objectstack/core` — the plugin's own + * `import type { Plugin, PluginContext }` is erased before resolution and so was + * never a hazard — and an unaliased value import resolves through `exports` to + * `core/dist`, which would make the verdict a function of build state rather + * than of the source in the checkout. + * + * `pnpm check:test-source-alias` reds on exactly that and names this remedy: + * alias it to source rather than widen `KNOWN_UNALIASED_TEST_IMPORTS`, which is + * shrink-only. + * + * ANCHORED regex, array form, deliberately. A bare string `find` matches by + * PREFIX, so with a FILE replacement it would also swallow the published + * `@objectstack/core/logger` subpath and resolve it to + * `…/core/src/index.ts/logger` — `ENOTDIR`, at run time, from a config that + * reads as correct. Anchoring leaves that subpath to `exports`, where it + * belongs. + * + * `test` is deliberately left unset: this package's `test` script is a bare + * `vitest run` and was relying on the defaults, so declaring any of them here + * would silently narrow what the suite collects. + */ +export default defineConfig({ + resolve: { + alias: [ + { find: /^@objectstack\/core$/, replacement: path.resolve(__dirname, '../../core/src/index.ts') }, + ], + }, +}); diff --git a/scripts/check-plugin-teardown-shape.mjs b/scripts/check-plugin-teardown-shape.mjs index 37131210d2..5e93066ba7 100644 --- a/scripts/check-plugin-teardown-shape.mjs +++ b/scripts/check-plugin-teardown-shape.mjs @@ -130,11 +130,12 @@ * * ## The known list * - * `KNOWN_TEARDOWN_UNREACHED` baselines the instances that existed when this - * gate landed. Their repair is #10371's, in the services lane; repairing them - * here would have made this PR unreviewable against its own card. The list is - * a ratchet that only shrinks -- an entry is deleted when its plugin is - * repaired, and a stale entry is itself a failure. + * `KNOWN_TEARDOWN_UNREACHED` baselined the instances that existed when this + * gate landed. It is a ratchet that only shrinks -- an entry is deleted when + * its plugin is repaired, and a stale entry is itself a failure. As of #10772 + * it is EMPTY: every baselined instance has been repaired (six under #10371, + * the remaining five under #10772), so the gate now judges the whole + * population with no exemptions at all. */ import { spawnSync } from 'node:child_process'; @@ -219,22 +220,30 @@ const POSITIVE_CONTROL = { /** * Every `Plugin` implementation that declared a teardown alias and no - * `destroy()` when this gate landed. Their repair is #10371's card, in the - * services lane. + * `destroy()` when this gate landed -- and, since #10772, NONE OF THEM. * * ⛔ SHRINK-ONLY. The list only ever shrinks: an entry is DELETED when its * plugin grows a real `destroy()`, and a stale entry fails this gate. It is * closed to new entries -- see the failure text. * - * Six of these are the instances #10371 enumerates. Five are not, and were - * derived here rather than adopted from that card: `MetadataPlugin`, - * `AppPlugin` and `ExternalValidationPlugin` spell the alias as an arrow - * PROPERTY (which a method-only reading of the class misses), and - * `EmailServicePlugin` / `WebhookOutboxPlugin` spell it `dispose`. + * The burn-down, so the empty array is a MEASURED state and not an abandoned + * one. Eleven entries landed with the gate. Six were the instances #10371 + * enumerates, and #10371 repaired them. The other five were derived here + * rather than adopted from that card, filed as #10772 and repaired there: + * `MetadataPlugin`, `AppPlugin` and `ExternalValidationPlugin` spelled the + * alias as an arrow PROPERTY (which a method-only reading of the class + * misses -- that is precisely why #10371's own enumeration was short by + * five), and `EmailServicePlugin` / `WebhookOutboxPlugin` spelled it + * `dispose` -- the "seventh spelling" this gate's roster was widened for + * before any instance of it existed, already present when the roster was + * measured. + * + * An empty ratchet is the state this gate exists to reach, not a reason to + * delete it: the roster, the population and the refusals are what keep the + * class closed for the NEXT instance, and the self-test's live-tree case + * ("neither short nor stale") is what keeps this array honest either way. */ -const KNOWN_TEARDOWN_UNREACHED = [ - { file: 'packages/plugins/plugin-webhooks/src/webhook-outbox-plugin.ts', cls: 'WebhookOutboxPlugin', alias: 'dispose', repair: '#10371' }, -]; +const KNOWN_TEARDOWN_UNREACHED = []; // --------------------------------------------------------------------------- // The scan @@ -454,9 +463,10 @@ function main() { + `\n async ${TEARDOWN_ALIASES[0]}(): Promise { await this.${KERNEL_HOOK}(); }` + '\n' + '\n ⛔ Do not add an entry to KNOWN_TEARDOWN_UNREACHED to get past this. That' - + '\n list is shrink-only and closed to new entries: it baselines the instances' - + '\n that predate this gate, whose repair belongs to #10371, and widening it' - + '\n is not a fix -- it would reopen the class this gate exists to close.', + + '\n list is shrink-only and closed to new entries: it baselined the instances' + + '\n that predate this gate, and since #10772 it is EMPTY -- every one of them' + + '\n has been repaired. Widening it is not a fix; it would reopen the class' + + '\n this gate exists to close, and it would be the first entry back.', ); return 1; } @@ -475,10 +485,12 @@ function main() { return 1; } + const repairCards = [...new Set(KNOWN_TEARDOWN_UNREACHED.map((k) => k.repair))].sort(); console.log( `✓ check:plugin-teardown-shape: ${classes} Plugin implementation(s) across ${files} source(s) under ${POPULATION}; ` + `every teardown-shaped method (${TEARDOWN_ALIASES.join(' / ')}) sits beside a real ${KERNEL_HOOK}() ` - + `(${held} known-unreached, ⛔ SHRINK-ONLY, repair tracked on #10371).`, + + `(${held} known-unreached, ⛔ SHRINK-ONLY` + + `${repairCards.length ? `, repair tracked on ${repairCards.join(' / ')}` : ', baseline fully burned down'}).`, ); return 0; } From f2876600263bbfbe7e3da2a5d9d35c94ced6e916 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 14:32:49 +0000 Subject: [PATCH 6/7] chore(changeset): patch for the five #10772 teardown repairs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Patch, not minor: nothing is removed, no signature narrows, and the `Plugin` interface is untouched. Every old spelling is retained as a delegating alias with an identical or WIDENED signature (`stop(ctx)` -> `stop(_ctx?)`), so no `ADR-0087` conversion-layer marker is owed. The one behavioural difference for direct callers — the two `stop(ctx)` aliases now log/emit through the context captured in `init()` rather than the argument — is named in the changeset body rather than left for a reader to discover. Committed BEFORE the gate union is run on purpose: `check-adr-0087-registration` reads changeset content from GIT, not the working tree, so running it against an uncommitted changeset returns a green that carries no information. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx --- .../plugin-teardown-reached-by-kernel.md | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 .changeset/plugin-teardown-reached-by-kernel.md diff --git a/.changeset/plugin-teardown-reached-by-kernel.md b/.changeset/plugin-teardown-reached-by-kernel.md new file mode 100644 index 0000000000..3bc89ff57a --- /dev/null +++ b/.changeset/plugin-teardown-reached-by-kernel.md @@ -0,0 +1,47 @@ +--- +"@objectstack/metadata": patch +"@objectstack/runtime": patch +"@objectstack/plugin-email": patch +"@objectstack/plugin-webhooks": patch +--- + +Five `Plugin` implementations now release their resources from `destroy()`, the +only teardown hook the kernel calls (#10772). + +`Plugin` (`@objectstack/core`'s `types.ts`) declares `init()`, `start?(ctx)` and +`destroy?()`. `ObjectKernel.performShutdown()` and `LiteKernel.destroy()` walk +the plugins in reverse calling `plugin.destroy()` — and nothing anywhere calls +`stop()`, `dispose()`, `close()` or `shutdown()` on a plugin. Each of these five +spelled its teardown with one of those names instead, so what it released was +still held after `await kernel.shutdown()` had **resolved**: + +| package | class | was spelled | what outlived shutdown | +|:--|:--|:--|:--| +| `@objectstack/metadata` | `MetadataPlugin` | `stop` (arrow property) | artifact watcher, `manager.dispose()`, repository handle | +| `@objectstack/runtime` | `AppPlugin` | `stop` (arrow property) | the `app:unregistered` catalog event, never emitted | +| `@objectstack/runtime` | `ExternalValidationPlugin` | `stop` (arrow property) | every armed drift-check `setInterval` | +| `@objectstack/plugin-email` | `EmailServicePlugin` | `dispose` | two metadata subscriptions, the SMTP transport, an engine binding | +| `@objectstack/plugin-webhooks` | `WebhookOutboxPlugin` | `dispose` | the auto-enqueuer (2 realtime subscriptions + a refresh interval) and two engine hooks | + +`ExternalValidationPlugin` is the one with teeth: it is one of only two `Plugin` +implementations in the tree that own `setInterval` directly, it is mounted on +the real `os serve` path, and its `stop()`'s only caller anywhere was the class +itself re-arming. Measured against a real kernel, its drift checker performed +five further reads in the five intervals after a resolved shutdown — the #9371 +mechanism verbatim. `WebhookOutboxPlugin.dispose()` had **zero** callers in the +entire repo, so its teardown had never run in any process at all. + +**Nothing is removed and no signature narrows.** Each old name is retained as a +delegating alias, because it is public API of an exported class and an embedder +may have learned to call it directly precisely BECAUSE the kernel never did. +`stop` stays an arrow property where it was one (so a detached +`const { stop } = plugin` keeps working) and stays synchronous on +`ExternalValidationPlugin` (so a non-awaiting call site is unaffected). The two +`stop(ctx)` aliases widen their parameter to optional. + +One behavioural note for direct callers, since `destroy()` takes no context: +`MetadataPlugin.stop(ctx)` and `AppPlugin.stop(ctx)` now use the context +captured in `init()` and ignore the argument. In a real composition these are +the same object. The visible difference is confined to a plugin whose `init()` +never ran — for `MetadataPlugin` a dropped `warn` line, for `AppPlugin` a +catalog event that is no longer emitted for an app that was never registered. From 66d2bfe96f685df2f007b6b671d5711d23575371 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 14:48:22 +0000 Subject: [PATCH 7/7] test(webhooks,email): satisfy check:engine-double-contract for the two new fakes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both findings came from the gate's own verdict, not from a guess: x PINNED [delete]/[update]: plugin-shutdown-stops-auto-enqueuer.test.ts declares 1 engine double whose delete()/update() does not route through assertEngineDeleteDispatch/assertEngineUpdateDispatch (line 67). x RETAINED [update]: plugin-shutdown-detaches-template-bridge.test.ts pins 1 engine double that the pinned ledger does not record. The webhook double is fixed by DELETING the two verbs rather than pinning them. Nothing on the path under test writes through `update()` or `delete()`, so they were a double looser than `ObjectQL` serving no call — which is the shape this gate exists to refuse. Removing them takes the double out of the population instead of adding a row to a shrink-only ledger. ⛔ The baseline in `scripts/engine-double-contract.baseline.json` is untouched: that path is maintainer-only and the gate says so. The email double already routes through `assertEngineUpdateDispatch` (it is the same double `email-plugin.template-runtime-write.test.ts` uses); only the RETAINED ledger had to learn about it, via the prescribed `node scripts/check-engine-double-contract.mjs --write`. That regeneration reported "1 added or grown, 0 lost" — new pinned coverage, no pin weakened. After: `check-engine-double-contract: OK — 372 pinned, 133 in the DEBT ledger, 2 exempt.` plugin-webhooks suite 124/124. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx --- .../src/plugin-shutdown-stops-auto-enqueuer.test.ts | 6 ++++-- scripts/engine-double-contract.pinned.json | 5 +++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/plugins/plugin-webhooks/src/plugin-shutdown-stops-auto-enqueuer.test.ts b/packages/plugins/plugin-webhooks/src/plugin-shutdown-stops-auto-enqueuer.test.ts index 7dfbd9bf23..3de1913e35 100644 --- a/packages/plugins/plugin-webhooks/src/plugin-shutdown-stops-auto-enqueuer.test.ts +++ b/packages/plugins/plugin-webhooks/src/plugin-shutdown-stops-auto-enqueuer.test.ts @@ -68,8 +68,10 @@ function fakeEngine() { registry: { listItems: () => [], getObject: () => undefined }, async find() { return []; }, async insert(_o: string, row: AnyRecord) { return { id: 'row_1', ...row }; }, - async update() { return { affected: 0 }; }, - async delete() { return true; }, + // No `update()` / `delete()` on purpose. Nothing on the path under + // test writes through them, and a double that declares a write verb + // it never serves is a double looser than `ObjectQL` for no reason — + // which is the shape `check:engine-double-contract` exists to refuse. registerHook() { /* provenance stamp + headers gate */ }, unregisterHooksByPackage() { return 0; }, }; diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index c79605ac78..3469e93fcf 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -1336,6 +1336,11 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/plugins/plugin-email/src/plugin-shutdown-detaches-template-bridge.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/plugins/plugin-security/src/authored-row-write-verdict.test.ts", "verb": "delete",