Skip to content
Merged
47 changes: 47 additions & 0 deletions .changeset/plugin-teardown-reached-by-kernel.md
Original file line numberDiff line numberDiff line change
@@ -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.
175 changes: 175 additions & 0 deletions packages/metadata/src/plugin-shutdown-releases-repository.test.ts
Original file line numberDiff line numberDiff line change
@@ -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<void> } | 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<void> }): () => 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();
});
});
42 changes: 40 additions & 2 deletions packages/metadata/src/plugin.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -238,6 +238,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<void> };
/**
* 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`
Expand DownExpand Up@@ -283,6 +290,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,
Expand DownExpand Up@@ -546,15 +556,30 @@ 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<void> => {
if (this.artifactWatcher) {
try { await this.artifactWatcher.close(); } catch { /* noop */ }
this.artifactWatcher = undefined;
}
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') {
Expand All@@ -563,6 +588,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<void> => {
await this.destroy();
}

/**
* Fetch JSON content from a URL with configurable timeout.
*/
Expand Down
28 changes: 27 additions & 1 deletion packages/plugins/plugin-email/src/email-plugin.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -1252,7 +1252,23 @@ export class EmailServicePlugin implements Plugin {
return stripReadDecorations(item);
}

async dispose(): Promise<void> {
/**
* 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<void> {
this.templateBridgeArmed = false;
try { this.unsubscribeTemplates?.(); } catch { /* best effort */ }
this.unsubscribeTemplates = undefined;
Expand All@@ -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<void> {
await this.destroy();
}

/**
* Translate the `mail` settings namespace snapshot into a transport
* and `defaultFrom`, then hot-swap them on the running EmailService.
Expand Down
Loading
Loading