Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .changeset/plugin-teardown-reaches-destroy.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
---
"@objectstack/plugin-reports": patch
"@objectstack/connector-openapi": patch
"@objectstack/connector-rest": patch
"@objectstack/connector-slack": patch
"@objectstack/plugin-approvals": patch
"@objectstack/service-knowledge": patch
---

Release these plugins' resources from `destroy()`, the teardown hook the kernel
actually calls (#10371). `Plugin` 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 every plugin whose teardown was spelled
`stop()`. `await kernel.shutdown()` resolved with the reports dispatcher still
armed, the REST/OpenAPI/Slack connectors still registered on the automation
engine, the approvals SLA escalation job still scheduled, and the knowledge
event-sync subscription still open.

Each teardown body now lives in `destroy()`. `stop()` is retained as a
delegating alias with its parameter made optional, so an embedder that learned
to call it directly — precisely because the kernel never did — keeps working
unchanged. No export is removed and the `Plugin` interface is untouched.

Same defect as #9371 in `@objectstack/service-messaging`, which surfaced as
fully green test runs exiting 1 on `EnvironmentTeardownError` and being evicted
from the merge queue.
Original file line numberDiff line numberDiff line change
Expand Up@@ -101,10 +101,38 @@ export class ConnectorOpenApiPlugin implements Plugin {
ctx.logger.info(`ConnectorOpenApiPlugin: OpenAPI connector '${this.connectorName}' registered`);
}

async stop(_ctx: PluginContext): Promise<void> {
/**
* The kernel's teardown hook (`Plugin.destroy?()`, core `types.ts`) — the
* ONLY teardown entry point `ObjectKernel.performShutdown()` and
* `LiteKernel.destroy()` invoke.
*
* [#10371] IT USED TO BE `stop()`, WHICH NOTHING CALLED. `Plugin` declares
* `init()`, `start?()` and `destroy?()` and no `stop()`, so the kernel
* walked past this plugin at shutdown and the OpenAPI connector stayed registered in the automation
* engine for the lifetime of the process. `start()` IS on the
* interface, so the pair read as symmetric in review — that asymmetry is
* what let the same shape survive in six packages at once.
*
* No timers here, so this instance never cost a merge-queue eviction the
* way the `plugin-reports` / `service-messaging` members did (#9371). The
* class is the same one either way: a teardown the kernel does not reach.
*/
async destroy(): Promise<void> {
if (this.automation && this.connectorName) {
try { this.automation.unregisterConnector(this.connectorName); } catch { /* ignore */ }
}
this.automation = undefined;
this.connectorName = undefined;
}

/**
* Retained alias for {@link destroy}. Kept because it is public API of an
* exported class, and removing it would break an embedder who learned to
* call it directly precisely BECAUSE the kernel never did. Prefer kernel
* shutdown; direct callers keep working unchanged.
*/
async stop(_ctx?: PluginContext): Promise<void> {
await this.destroy();
}

private tryGetAutomation(ctx: PluginContext): ConnectorRegistrySurface | undefined {
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* [#10371] `await kernel.shutdown()` must actually reach this plugin's teardown.
*
* THE DEFECT THIS PINS. The teardown that unregisters the hand-wired OpenAPI
* connector was spelled `stop()`. `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()`.
*
* WHY THE ASSERTION IS 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.
*
* THE PRE-SHUTDOWN LEG IS A POSITIVE CONTROL and load-bearing: without it, a
* plugin that never registered anything would satisfy the post-shutdown
* assertion vacuously.
*/

import { describe, it, expect } from 'vitest';
import { LiteKernel } from '@objectstack/core';
import { AutomationServicePlugin, type AutomationEngine } from '@objectstack/service-automation';
import { ConnectorOpenApiPlugin } from './connector-openapi-plugin.js';
import type { OpenApiDocument } from './openapi-connector.js';

/** Smallest document that yields one named connector ('mini') with one action. */
const document: OpenApiDocument = {
info: { title: 'Mini' },
servers: [{ url: 'https://api.mini.example.com' }],
paths: {
'/ping': { get: { operationId: 'ping', responses: { '200': { description: 'ok' } } } },
},
};

describe('#10371 ConnectorOpenApiPlugin releases its connector on kernel shutdown', () => {
it('unregisters the OpenAPI connector once shutdown() has resolved', async () => {
const kernel = new LiteKernel();
kernel.use(new AutomationServicePlugin());
kernel.use(new ConnectorOpenApiPlugin({ document }));
await kernel.bootstrap();

const engine = kernel.getService<AutomationEngine>('automation');

// POSITIVE CONTROL — the connector really is registered.
expect(engine.getRegisteredConnectors()).toContain('mini');

await kernel.shutdown();

// THE PIN. Before the fix this still contained 'mini'.
expect(engine.getRegisteredConnectors()).not.toContain('mini');
});

it('the retained stop() alias still tears down for an embedder that calls it directly', async () => {
const kernel = new LiteKernel();
kernel.use(new AutomationServicePlugin());
const plugin = new ConnectorOpenApiPlugin({ document });
kernel.use(plugin);
await kernel.bootstrap();

const engine = kernel.getService<AutomationEngine>('automation');
expect(engine.getRegisteredConnectors()).toContain('mini');

await plugin.stop();

expect(engine.getRegisteredConnectors()).not.toContain('mini');
});
});
59 changes: 59 additions & 0 deletions packages/connectors/connector-openapi/vitest.config.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* This package had NO vitest config until #10371, and that is the fact this
* header exists to keep visible: adding one changes how every test file in
* `packages/connectors/connector-openapi` is configured, not just the file that
* needed it. So it is deliberately minimal — anchored `resolve.alias` entries
* and **no `test` block at all**, because the package's existing test files run
* on vitest's defaults (`globals: false`, `environment: 'node'`) and import
* `describe`/`it`/`expect` explicitly. Sibling configs in this repo do carry
* `test: { globals: true, … }`; copying that shape here would silently
* re-specify the defaults for every existing file.
*
* ## Why the two entries
*
* `plugin-shutdown-unregisters-connector.test.ts` (#10371) boots a REAL
* `LiteKernel` with the REAL `AutomationServicePlugin` to prove that
* `kernel.shutdown()` reaches `ConnectorOpenApiPlugin.destroy()` — the whole
* point of that card is that the kernel calls `destroy()` and never called
* `stop()`, so a stand-in kernel would assert nothing. Without these entries
* both imports resolve through their packages' `exports` to **dist**, which
* makes the test a verdict about build state rather than about the source in
* the checkout — and the dangerous half of that is not a loud error but a test
* that passes GREEN against a stale artifact with nothing in the output saying
* so. `scripts/check-test-source-alias.mjs` carries the measured history
* (#7668, #7778, #7849); it named these two imports and is the gate that fails
* without the entries below.
*
* ⚠️ The registry in that script is SHRINK-ONLY, so widening
* `KNOWN_UNALIASED_TEST_IMPORTS['@objectstack/connector-openapi']` was never an
* option, and it is deliberately left untouched: its one remaining member,
* `@objectstack/spec`, is still reached unaliased by this package's other test
* files and stays that registry's problem to retire. Aliasing bare
* `@objectstack/spec` here would additionally hit rule 5's ENOTDIR trap (its
* subpaths would resolve through `…/spec/src/index.ts/<sub>`), which is why
* only the two specifiers the gate actually named are added.
*/

import { defineConfig } from 'vitest/config';
import path from 'path';

export default defineConfig({
resolve: {
// Array form with ANCHORED patterns, per the trap the gate documents: the
// object form matches by PREFIX, so a bare key whose replacement is a FILE
// also swallows every subpath and resolves it to `…/index.ts/<sub>`
// (`ENOTDIR`, at run time, in a config that looks right).
alias: [
{
find: /^@objectstack\/core$/,
replacement: path.resolve(__dirname, '../../core/src/index.ts'),
},
{
find: /^@objectstack\/service-automation$/,
replacement: path.resolve(__dirname, '../../services/service-automation/src/index.ts'),
},
],
},
});
30 changes: 29 additions & 1 deletion packages/connectors/connector-rest/src/connector-rest-plugin.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -92,10 +92,38 @@ export class ConnectorRestPlugin implements Plugin {
ctx.logger.info(`ConnectorRestPlugin: REST connector '${def.name}' registered`);
}

async stop(_ctx: PluginContext): Promise<void> {
/**
* The kernel's teardown hook (`Plugin.destroy?()`, core `types.ts`) — the
* ONLY teardown entry point `ObjectKernel.performShutdown()` and
* `LiteKernel.destroy()` invoke.
*
* [#10371] IT USED TO BE `stop()`, WHICH NOTHING CALLED. `Plugin` declares
* `init()`, `start?()` and `destroy?()` and no `stop()`, so the kernel
* walked past this plugin at shutdown and the REST connector stayed registered in the automation
* engine for the lifetime of the process. `start()` IS on the
* interface, so the pair read as symmetric in review — that asymmetry is
* what let the same shape survive in six packages at once.
*
* No timers here, so this instance never cost a merge-queue eviction the
* way the `plugin-reports` / `service-messaging` members did (#9371). The
* class is the same one either way: a teardown the kernel does not reach.
*/
async destroy(): Promise<void> {
if (this.automation && this.connectorName) {
try { this.automation.unregisterConnector(this.connectorName); } catch { /* ignore */ }
}
this.automation = undefined;
this.connectorName = undefined;
}

/**
* Retained alias for {@link destroy}. Kept because it is public API of an
* exported class, and removing it would break an embedder who learned to
* call it directly precisely BECAUSE the kernel never did. Prefer kernel
* shutdown; direct callers keep working unchanged.
*/
async stop(_ctx?: PluginContext): Promise<void> {
await this.destroy();
}

private tryGetAutomation(ctx: PluginContext): ConnectorRegistrySurface | undefined {
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* [#10371] `await kernel.shutdown()` must actually reach this plugin's teardown.
*
* THE DEFECT THIS PINS. The teardown that unregisters the REST connector was
* spelled `stop()`. `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()`.
*
* WHY THE ASSERTION IS 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 this drives a real kernel through a real shutdown and reads the automation
* engine's own connector registry.
*
* THE PRE-SHUTDOWN LEG IS A POSITIVE CONTROL and load-bearing: without it, a
* plugin that never registered anything would satisfy the post-shutdown
* assertion vacuously.
*/

import { describe, it, expect } from 'vitest';
import { LiteKernel } from '@objectstack/core';
import { AutomationServicePlugin, type AutomationEngine } from '@objectstack/service-automation';
import { ConnectorRestPlugin } from './connector-rest-plugin.js';

const options = { baseUrl: 'https://api.example.com' };

describe('#10371 ConnectorRestPlugin releases its connector on kernel shutdown', () => {
it('unregisters the REST connector once shutdown() has resolved', async () => {
const kernel = new LiteKernel();
kernel.use(new AutomationServicePlugin());
kernel.use(new ConnectorRestPlugin(options));
await kernel.bootstrap();

const engine = kernel.getService<AutomationEngine>('automation');

// POSITIVE CONTROL — the connector really is registered, so the
// assertion below measures removal and not absence.
expect(engine.getRegisteredConnectors()).toContain('rest');

await kernel.shutdown();

// THE PIN. Before the fix this still contained 'rest': the kernel had
// no `destroy()` to call and `stop()` was never anybody's business.
expect(engine.getRegisteredConnectors()).not.toContain('rest');
});

it('the retained stop() alias still tears down for an embedder that calls it directly', async () => {
// The alias exists precisely because an embedder may have learned to
// call it BECAUSE the kernel never did. Removing it would break them,
// so its behaviour is pinned rather than left to the fix's discretion.
const kernel = new LiteKernel();
kernel.use(new AutomationServicePlugin());
const plugin = new ConnectorRestPlugin(options);
kernel.use(plugin);
await kernel.bootstrap();

const engine = kernel.getService<AutomationEngine>('automation');
expect(engine.getRegisteredConnectors()).toContain('rest');

await plugin.stop();

expect(engine.getRegisteredConnectors()).not.toContain('rest');
});
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -71,9 +71,37 @@ export class ConnectorSlackPlugin implements Plugin {
ctx.logger.info(`ConnectorSlackPlugin: Slack connector '${def.name}' registered`);
}

async stop(_ctx: PluginContext): Promise<void> {
/**
* The kernel's teardown hook (`Plugin.destroy?()`, core `types.ts`) — the
* ONLY teardown entry point `ObjectKernel.performShutdown()` and
* `LiteKernel.destroy()` invoke.
*
* [#10371] IT USED TO BE `stop()`, WHICH NOTHING CALLED. `Plugin` declares
* `init()`, `start?()` and `destroy?()` and no `stop()`, so the kernel
* walked past this plugin at shutdown and the Slack connector stayed registered in the automation
* engine for the lifetime of the process. `start()` IS on the
* interface, so the pair read as symmetric in review — that asymmetry is
* what let the same shape survive in six packages at once.
*
* No timers here, so this instance never cost a merge-queue eviction the
* way the `plugin-reports` / `service-messaging` members did (#9371). The
* class is the same one either way: a teardown the kernel does not reach.
*/
async destroy(): Promise<void> {
if (this.automation && this.connectorName) {
try { this.automation.unregisterConnector(this.connectorName); } catch { /* ignore */ }
}
this.automation = undefined;
this.connectorName = undefined;
}

/**
* Retained alias for {@link destroy}. Kept because it is public API of an
* exported class, and removing it would break an embedder who learned to
* call it directly precisely BECAUSE the kernel never did. Prefer kernel
* shutdown; direct callers keep working unchanged.
*/
async stop(_ctx?: PluginContext): Promise<void> {
await this.destroy();
}
}
Loading
Loading