Skip to content

Commit 8aa5a31

Browse files
os-warrenclaude
andcommitted
fix(plugins,connectors,services): release plugin resources from destroy(), the hook the kernel calls
`Plugin` (packages/core/src/types.ts) declares `init()`, `start?(ctx)` and `destroy?()` — and no `stop()`. `ObjectKernel.performShutdown()` and `LiteKernel.destroy()` walk the plugins in reverse calling `plugin.destroy()`, so six plugins whose teardown was spelled `stop()` were never torn down: `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 moves into `destroy()`. `stop()` is retained as a delegating alias with an optional parameter, 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. No export is removed; the `Plugin` interface is untouched. `destroy()` takes no `PluginContext`, so the three members whose teardown used `ctx` now hold what they need from where they took it out: plugin-reports keeps the logger captured in `init()`, plugin-approvals keeps the `IJobService` the schedule was placed with, and service-knowledge keeps the `IRealtimeService` the subscription was taken out with. Same defect as #9371 in @objectstack/service-messaging, which surfaced as fully green runs exiting 1 on `EnvironmentTeardownError` and being evicted from the merge queue. connector-openapi gains a vitest.config.ts: `check:test-source-alias` requires the new test's `@objectstack/core` and `@objectstack/service-automation` imports to resolve to source, and its registry entry is shrink-only. Two anchored alias entries, no `test` block, so the package's existing files keep vitest defaults. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
1 parent 53a48c9 commit 8aa5a31

14 files changed

Lines changed: 1028 additions & 11 deletions
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
---
2+
"@objectstack/plugin-reports": patch
3+
"@objectstack/connector-openapi": patch
4+
"@objectstack/connector-rest": patch
5+
"@objectstack/connector-slack": patch
6+
"@objectstack/plugin-approvals": patch
7+
"@objectstack/service-knowledge": patch
8+
---
9+
10+
Release these plugins' resources from `destroy()`, the teardown hook the kernel
11+
actually calls (#10371). `Plugin` declares `init()`, `start?(ctx)` and
12+
`destroy?()` — and no `stop()` — so `ObjectKernel.performShutdown()` and
13+
`LiteKernel.destroy()`, which walk the plugins in reverse calling
14+
`plugin.destroy()`, walked straight past every plugin whose teardown was spelled
15+
`stop()`. `await kernel.shutdown()` resolved with the reports dispatcher still
16+
armed, the REST/OpenAPI/Slack connectors still registered on the automation
17+
engine, the approvals SLA escalation job still scheduled, and the knowledge
18+
event-sync subscription still open.
19+
20+
Each teardown body now lives in `destroy()`. `stop()` is retained as a
21+
delegating alias with its parameter made optional, so an embedder that learned
22+
to call it directly — precisely because the kernel never did — keeps working
23+
unchanged. No export is removed and the `Plugin` interface is untouched.
24+
25+
Same defect as #9371 in `@objectstack/service-messaging`, which surfaced as
26+
fully green test runs exiting 1 on `EnvironmentTeardownError` and being evicted
27+
from the merge queue.

‎packages/connectors/connector-openapi/src/connector-openapi-plugin.ts‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,10 +101,38 @@ export class ConnectorOpenApiPlugin implements Plugin {
101101
ctx.logger.info(`ConnectorOpenApiPlugin: OpenAPI connector '${this.connectorName}' registered`);
102102
}
103103

104-
async stop(_ctx: PluginContext): Promise<void> {
104+
/**
105+
* The kernel's teardown hook (`Plugin.destroy?()`, core `types.ts`) — the
106+
* ONLY teardown entry point `ObjectKernel.performShutdown()` and
107+
* `LiteKernel.destroy()` invoke.
108+
*
109+
* [#10371] IT USED TO BE `stop()`, WHICH NOTHING CALLED. `Plugin` declares
110+
* `init()`, `start?()` and `destroy?()` and no `stop()`, so the kernel
111+
* walked past this plugin at shutdown and the OpenAPI connector stayed registered in the automation
112+
* engine for the lifetime of the process. `start()` IS on the
113+
* interface, so the pair read as symmetric in review — that asymmetry is
114+
* what let the same shape survive in six packages at once.
115+
*
116+
* No timers here, so this instance never cost a merge-queue eviction the
117+
* way the `plugin-reports` / `service-messaging` members did (#9371). The
118+
* class is the same one either way: a teardown the kernel does not reach.
119+
*/
120+
async destroy(): Promise<void> {
105121
if (this.automation && this.connectorName) {
106122
try { this.automation.unregisterConnector(this.connectorName); } catch { /* ignore */ }
107123
}
124+
this.automation = undefined;
125+
this.connectorName = undefined;
126+
}
127+
128+
/**
129+
* Retained alias for {@link destroy}. Kept because it is public API of an
130+
* exported class, and removing it would break an embedder who learned to
131+
* call it directly precisely BECAUSE the kernel never did. Prefer kernel
132+
* shutdown; direct callers keep working unchanged.
133+
*/
134+
async stop(_ctx?: PluginContext): Promise<void> {
135+
await this.destroy();
108136
}
109137

110138
private tryGetAutomation(ctx: PluginContext): ConnectorRegistrySurface | undefined {
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#10371] `await kernel.shutdown()` must actually reach this plugin's teardown.
5+
*
6+
* THE DEFECT THIS PINS. The teardown that unregisters the hand-wired OpenAPI
7+
* connector was spelled `stop()`. `Plugin` (`@objectstack/core`'s `types.ts`)
8+
* declares `init()`, `start?(ctx)` and `destroy?()` — and NO `stop()` — so
9+
* `ObjectKernel.performShutdown()` and `LiteKernel.destroy()`, which walk the
10+
* plugins in reverse calling `plugin.destroy()`, walked straight past it.
11+
* Nothing in the repo ever called `stop()`.
12+
*
13+
* WHY THE ASSERTION IS BEHAVIOURAL. `expect(plugin.destroy).toBeDefined()`
14+
* would pass on a plugin the kernel still never reaches — the hook merely
15+
* EXISTING is not the property that was missing, being CALLED BY THE KERNEL is.
16+
*
17+
* THE PRE-SHUTDOWN LEG IS A POSITIVE CONTROL and load-bearing: without it, a
18+
* plugin that never registered anything would satisfy the post-shutdown
19+
* assertion vacuously.
20+
*/
21+
22+
import { describe, it, expect } from 'vitest';
23+
import { LiteKernel } from '@objectstack/core';
24+
import { AutomationServicePlugin, type AutomationEngine } from '@objectstack/service-automation';
25+
import { ConnectorOpenApiPlugin } from './connector-openapi-plugin.js';
26+
import type { OpenApiDocument } from './openapi-connector.js';
27+
28+
/** Smallest document that yields one named connector ('mini') with one action. */
29+
const document: OpenApiDocument = {
30+
info: { title: 'Mini' },
31+
servers: [{ url: 'https://api.mini.example.com' }],
32+
paths: {
33+
'/ping': { get: { operationId: 'ping', responses: { '200': { description: 'ok' } } } },
34+
},
35+
};
36+
37+
describe('#10371 ConnectorOpenApiPlugin releases its connector on kernel shutdown', () => {
38+
it('unregisters the OpenAPI connector once shutdown() has resolved', async () => {
39+
const kernel = new LiteKernel();
40+
kernel.use(new AutomationServicePlugin());
41+
kernel.use(new ConnectorOpenApiPlugin({ document }));
42+
await kernel.bootstrap();
43+
44+
const engine = kernel.getService<AutomationEngine>('automation');
45+
46+
// POSITIVE CONTROL — the connector really is registered.
47+
expect(engine.getRegisteredConnectors()).toContain('mini');
48+
49+
await kernel.shutdown();
50+
51+
// THE PIN. Before the fix this still contained 'mini'.
52+
expect(engine.getRegisteredConnectors()).not.toContain('mini');
53+
});
54+
55+
it('the retained stop() alias still tears down for an embedder that calls it directly', async () => {
56+
const kernel = new LiteKernel();
57+
kernel.use(new AutomationServicePlugin());
58+
const plugin = new ConnectorOpenApiPlugin({ document });
59+
kernel.use(plugin);
60+
await kernel.bootstrap();
61+
62+
const engine = kernel.getService<AutomationEngine>('automation');
63+
expect(engine.getRegisteredConnectors()).toContain('mini');
64+
65+
await plugin.stop();
66+
67+
expect(engine.getRegisteredConnectors()).not.toContain('mini');
68+
});
69+
});
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* This package had NO vitest config until #10371, and that is the fact this
5+
* header exists to keep visible: adding one changes how every test file in
6+
* `packages/connectors/connector-openapi` is configured, not just the file that
7+
* needed it. So it is deliberately minimal — anchored `resolve.alias` entries
8+
* and **no `test` block at all**, because the package's existing test files run
9+
* on vitest's defaults (`globals: false`, `environment: 'node'`) and import
10+
* `describe`/`it`/`expect` explicitly. Sibling configs in this repo do carry
11+
* `test: { globals: true, … }`; copying that shape here would silently
12+
* re-specify the defaults for every existing file.
13+
*
14+
* ## Why the two entries
15+
*
16+
* `plugin-shutdown-unregisters-connector.test.ts` (#10371) boots a REAL
17+
* `LiteKernel` with the REAL `AutomationServicePlugin` to prove that
18+
* `kernel.shutdown()` reaches `ConnectorOpenApiPlugin.destroy()` — the whole
19+
* point of that card is that the kernel calls `destroy()` and never called
20+
* `stop()`, so a stand-in kernel would assert nothing. Without these entries
21+
* both imports resolve through their packages' `exports` to **dist**, which
22+
* makes the test a verdict about build state rather than about the source in
23+
* the checkout — and the dangerous half of that is not a loud error but a test
24+
* that passes GREEN against a stale artifact with nothing in the output saying
25+
* so. `scripts/check-test-source-alias.mjs` carries the measured history
26+
* (#7668, #7778, #7849); it named these two imports and is the gate that fails
27+
* without the entries below.
28+
*
29+
* ⚠️ The registry in that script is SHRINK-ONLY, so widening
30+
* `KNOWN_UNALIASED_TEST_IMPORTS['@objectstack/connector-openapi']` was never an
31+
* option, and it is deliberately left untouched: its one remaining member,
32+
* `@objectstack/spec`, is still reached unaliased by this package's other test
33+
* files and stays that registry's problem to retire. Aliasing bare
34+
* `@objectstack/spec` here would additionally hit rule 5's ENOTDIR trap (its
35+
* subpaths would resolve through `…/spec/src/index.ts/<sub>`), which is why
36+
* only the two specifiers the gate actually named are added.
37+
*/
38+
39+
import { defineConfig } from 'vitest/config';
40+
import path from 'path';
41+
42+
export default defineConfig({
43+
resolve: {
44+
// Array form with ANCHORED patterns, per the trap the gate documents: the
45+
// object form matches by PREFIX, so a bare key whose replacement is a FILE
46+
// also swallows every subpath and resolves it to `…/index.ts/<sub>`
47+
// (`ENOTDIR`, at run time, in a config that looks right).
48+
alias: [
49+
{
50+
find: /^@objectstack\/core$/,
51+
replacement: path.resolve(__dirname, '../../core/src/index.ts'),
52+
},
53+
{
54+
find: /^@objectstack\/service-automation$/,
55+
replacement: path.resolve(__dirname, '../../services/service-automation/src/index.ts'),
56+
},
57+
],
58+
},
59+
});

‎packages/connectors/connector-rest/src/connector-rest-plugin.ts‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,38 @@ export class ConnectorRestPlugin implements Plugin {
9292
ctx.logger.info(`ConnectorRestPlugin: REST connector '${def.name}' registered`);
9393
}
9494

95-
async stop(_ctx: PluginContext): Promise<void> {
95+
/**
96+
* The kernel's teardown hook (`Plugin.destroy?()`, core `types.ts`) — the
97+
* ONLY teardown entry point `ObjectKernel.performShutdown()` and
98+
* `LiteKernel.destroy()` invoke.
99+
*
100+
* [#10371] IT USED TO BE `stop()`, WHICH NOTHING CALLED. `Plugin` declares
101+
* `init()`, `start?()` and `destroy?()` and no `stop()`, so the kernel
102+
* walked past this plugin at shutdown and the REST connector stayed registered in the automation
103+
* engine for the lifetime of the process. `start()` IS on the
104+
* interface, so the pair read as symmetric in review — that asymmetry is
105+
* what let the same shape survive in six packages at once.
106+
*
107+
* No timers here, so this instance never cost a merge-queue eviction the
108+
* way the `plugin-reports` / `service-messaging` members did (#9371). The
109+
* class is the same one either way: a teardown the kernel does not reach.
110+
*/
111+
async destroy(): Promise<void> {
96112
if (this.automation && this.connectorName) {
97113
try { this.automation.unregisterConnector(this.connectorName); } catch { /* ignore */ }
98114
}
115+
this.automation = undefined;
116+
this.connectorName = undefined;
117+
}
118+
119+
/**
120+
* Retained alias for {@link destroy}. Kept because it is public API of an
121+
* exported class, and removing it would break an embedder who learned to
122+
* call it directly precisely BECAUSE the kernel never did. Prefer kernel
123+
* shutdown; direct callers keep working unchanged.
124+
*/
125+
async stop(_ctx?: PluginContext): Promise<void> {
126+
await this.destroy();
99127
}
100128

101129
private tryGetAutomation(ctx: PluginContext): ConnectorRegistrySurface | undefined {
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#10371] `await kernel.shutdown()` must actually reach this plugin's teardown.
5+
*
6+
* THE DEFECT THIS PINS. The teardown that unregisters the REST connector was
7+
* spelled `stop()`. `Plugin` (`@objectstack/core`'s `types.ts`) declares
8+
* `init()`, `start?(ctx)` and `destroy?()` — and NO `stop()` — so
9+
* `ObjectKernel.performShutdown()` and `LiteKernel.destroy()`, which walk the
10+
* plugins in reverse calling `plugin.destroy()`, walked straight past it.
11+
* Nothing in the repo ever called `stop()`.
12+
*
13+
* WHY THE ASSERTION IS BEHAVIOURAL. `expect(plugin.destroy).toBeDefined()`
14+
* would pass on a plugin the kernel still never reaches — the hook merely
15+
* EXISTING is not the property that was missing, being CALLED BY THE KERNEL is.
16+
* So this drives a real kernel through a real shutdown and reads the automation
17+
* engine's own connector registry.
18+
*
19+
* THE PRE-SHUTDOWN LEG IS A POSITIVE CONTROL and load-bearing: without it, a
20+
* plugin that never registered anything would satisfy the post-shutdown
21+
* assertion vacuously.
22+
*/
23+
24+
import { describe, it, expect } from 'vitest';
25+
import { LiteKernel } from '@objectstack/core';
26+
import { AutomationServicePlugin, type AutomationEngine } from '@objectstack/service-automation';
27+
import { ConnectorRestPlugin } from './connector-rest-plugin.js';
28+
29+
const options = { baseUrl: 'https://api.example.com' };
30+
31+
describe('#10371 ConnectorRestPlugin releases its connector on kernel shutdown', () => {
32+
it('unregisters the REST connector once shutdown() has resolved', async () => {
33+
const kernel = new LiteKernel();
34+
kernel.use(new AutomationServicePlugin());
35+
kernel.use(new ConnectorRestPlugin(options));
36+
await kernel.bootstrap();
37+
38+
const engine = kernel.getService<AutomationEngine>('automation');
39+
40+
// POSITIVE CONTROL — the connector really is registered, so the
41+
// assertion below measures removal and not absence.
42+
expect(engine.getRegisteredConnectors()).toContain('rest');
43+
44+
await kernel.shutdown();
45+
46+
// THE PIN. Before the fix this still contained 'rest': the kernel had
47+
// no `destroy()` to call and `stop()` was never anybody's business.
48+
expect(engine.getRegisteredConnectors()).not.toContain('rest');
49+
});
50+
51+
it('the retained stop() alias still tears down for an embedder that calls it directly', async () => {
52+
// The alias exists precisely because an embedder may have learned to
53+
// call it BECAUSE the kernel never did. Removing it would break them,
54+
// so its behaviour is pinned rather than left to the fix's discretion.
55+
const kernel = new LiteKernel();
56+
kernel.use(new AutomationServicePlugin());
57+
const plugin = new ConnectorRestPlugin(options);
58+
kernel.use(plugin);
59+
await kernel.bootstrap();
60+
61+
const engine = kernel.getService<AutomationEngine>('automation');
62+
expect(engine.getRegisteredConnectors()).toContain('rest');
63+
64+
await plugin.stop();
65+
66+
expect(engine.getRegisteredConnectors()).not.toContain('rest');
67+
});
68+
});

‎packages/connectors/connector-slack/src/connector-slack-plugin.ts‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,37 @@ export class ConnectorSlackPlugin implements Plugin {
7171
ctx.logger.info(`ConnectorSlackPlugin: Slack connector '${def.name}' registered`);
7272
}
7373

74-
async stop(_ctx: PluginContext): Promise<void> {
74+
/**
75+
* The kernel's teardown hook (`Plugin.destroy?()`, core `types.ts`) — the
76+
* ONLY teardown entry point `ObjectKernel.performShutdown()` and
77+
* `LiteKernel.destroy()` invoke.
78+
*
79+
* [#10371] IT USED TO BE `stop()`, WHICH NOTHING CALLED. `Plugin` declares
80+
* `init()`, `start?()` and `destroy?()` and no `stop()`, so the kernel
81+
* walked past this plugin at shutdown and the Slack connector stayed registered in the automation
82+
* engine for the lifetime of the process. `start()` IS on the
83+
* interface, so the pair read as symmetric in review — that asymmetry is
84+
* what let the same shape survive in six packages at once.
85+
*
86+
* No timers here, so this instance never cost a merge-queue eviction the
87+
* way the `plugin-reports` / `service-messaging` members did (#9371). The
88+
* class is the same one either way: a teardown the kernel does not reach.
89+
*/
90+
async destroy(): Promise<void> {
7591
if (this.automation && this.connectorName) {
7692
try { this.automation.unregisterConnector(this.connectorName); } catch { /* ignore */ }
7793
}
94+
this.automation = undefined;
95+
this.connectorName = undefined;
96+
}
97+
98+
/**
99+
* Retained alias for {@link destroy}. Kept because it is public API of an
100+
* exported class, and removing it would break an embedder who learned to
101+
* call it directly precisely BECAUSE the kernel never did. Prefer kernel
102+
* shutdown; direct callers keep working unchanged.
103+
*/
104+
async stop(_ctx?: PluginContext): Promise<void> {
105+
await this.destroy();
78106
}
79107
}

0 commit comments

Comments
 (0)