Uh oh!
There was an error while loading. Please reload this page.
fix(plugins,connectors,services): release plugin resources from destroy(), the hook the kernel actually calls - #10766
Conversation
…oy(), 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
…ernelConfig `logLevel` is not a member of `ObjectKernelConfig` / `LiteKernel`'s config — both take `logger?: Partial<LoggerConfig>`. `tsc --noEmit` reads these test files, so the wrong spelling was a TS2353, and the `as any` on the LiteKernel call was hiding the same mistake rather than typing anything. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
📓 Docs Drift CheckThis PR changes 6 package(s): 3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 10 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 1b739d5d3efd713c8a9e80aaac97a9f479b3c2d9 && git checkout 1b739d5d3efd713c8a9e80aaac97a9f479b3c2d9
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 5f2e54cc66330cbc53a17f6e3746acdfcdc14704 0ba6c2a5bda0eee8564c21d28bc5dfc28593db19 && git checkout -B drift-repro 5f2e54cc66330cbc53a17f6e3746acdfcdc14704 && git merge --no-ff 0ba6c2a5bda0eee8564c21d28bc5dfc28593db19
node scripts/docs-audit/affected-docs.mjs --json 5f2e54cc66330cbc53a17f6e3746acdfcdc14704
|
…nsion `tsconfig.json` here is `include: ["src"]` under NodeNext, so this file is in the package's tsc program, and an extension-less relative import is a TS2835. `@objectstack/service-knowledge` carries a frozen 10-error DEBT entry that `check:type-check-debt --re-measure` re-measures, so the new error would have taken it to 11 and failed a shrink-only ratchet. Measured back at exactly 10 after this change. The neighbouring test file spells it the same extension-less way and is one of the two ledgered TS2835s there; copying it was the mistake. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
…ACHED `scripts/check-plugin-teardown-shape.mjs` (#10619) landed on main after this branch was cut. Its baseline is shrink-only and a stale entry is itself a failure, so the six plugins repaired here would have reddened the gate on main the moment this merged — the merge queue would more likely have evicted this PR first, rebuilding the queue for everyone behind it. That is the #9371 blast-radius shape, on a PR whose subject is #9371's defect class. Which six was taken from the gate's own failure output, not from line numbers: run before the edit, it named exactly these six as `no longer unreached`, and each was independently confirmed to declare a real `destroy()` in this tree. The other five entries are untouched. `MetadataPlugin`, `AppPlugin` and `ExternalValidationPlugin` spell the alias as an arrow property and `EmailServicePlugin` / `WebhookOutboxPlugin` spell it `dispose`; none has a `destroy()` yet, and they are #10772's card. Deleting them would be a false declaration that they are fixed. Held count after this change is 5, measured, not predicted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
Uh oh!
There was an error while loading. Please reload this page.
Fixes#10371
What was wrong
Plugin(packages/core/src/types.ts) declares exactly three lifecycle hooks:(each returning a void-or-promise-of-void; the angle brackets are left out
because this repo's issue-body sanitizer eats short bracketed fragments)
There is no
stop().ObjectKernel.performShutdown()(kernel.ts:771-783) andLiteKernel.destroy()(lite-kernel.ts:168-172) walk the plugins in reverse callingplugin.destroy(), so six plugins that spelled their teardownstop()were never torndown at all.
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.
The asymmetry is what hid it:
start?(ctx)is on the interface and does fire, so astart/stoppair reads as symmetric in review. Same defect as #9371 in@objectstack/service-messaging, which was not found by review or by a gate — it wasfound because it evicted two fully green PRs (#9365, #9775) from the merge queue on
EnvironmentTeardownError.The repair
Each teardown body moves into
destroy().stop()is retained as a delegating aliaswith its parameter made optional, 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
Plugininterface is untouched.destroy()takes noPluginContext, so the three members whose teardown usedctxnowhold what they need from where they took it out:
plugin-reportsctx.logger, captured ininit()plugin-approvalsIJobServicethe schedule was placed withservice-knowledgeIRealtimeServicethe subscription was taken out withMeasurements
1.
stop()really has no callers — a zero-hit claim, with its controlsplugin.destroy()— 10 call sites, four of them inpackages/core(kernel.ts:718,kernel.ts:776,kernel-base.ts:242,health-monitor.ts:233).stop(as a pattern — 192 hits acrosspackages/,examples/,apps/,scripts/.stop(calls on any of the six plugin instances; 0.stop(anywhere inside the six owning packages; 0 references to astopplugin hook anywhere inpackages/core/srcThe only near-miss the search surfaced is
stack?.stop()in two dogfood tests — that isVerifyStack.stop(packages/verify/src/harness.ts:667), which closes the HTTP serverand calls
kernel.shutdown(). It reachesdestroy(), never a plugin'sstop().2. Two premises in the card are wrong, and one matters
plugin-reportshas ONE timer site, not five. The card says "5setInterval/setTimeoutsites". A grep for those two identifiers inreports-plugin.tsdoes return five lines — but four of them are a doc comment (l.35),a type annotation naming
setInterval's return type (l.53), a fall-through comment(l.154) and a log string (l.165). There is exactly one timer creation, at l.156. Nothing
else in
packages/plugins/plugin-reports/src/calls a timer API at all.unref()ed (l.164), so the card's "at least some" is "the onlyone". The visible consequence today is therefore exactly the #9371 one — silent in a
long-lived host, expensive in a vitest worker — plus the job-service branch, which is not
a timer at all: with
service-jobinstalled the dispatcher is a scheduled job and theleak is an uncancelled job rather than an armed interval. Both branches are pinned.
Neither correction changes the defect or the repair.
3. The pins, by direction and by member
Every pre-shutdown leg is a load-bearing positive control: without it a plugin that
started nothing would satisfy the post-shutdown assertion vacuously.
shutdown()releases itstop()still works for a direct callerdestroy()reached by the kernelplugin-reportssys_report_schedulereads aftershutdown()resolves (real 5s clock, real ObjectQL +SqlDriver) andreports.dispatchcancelled on the job branchstop()andstop(ctx)both tear downconnector-reststop()tears downgetRegisteredConnectors()losesrestaftershutdown()connector-slackstop()tears downslackconnector-openapistop()tears downminiplugin-approvalsstop()tears downshutdown()service-knowledgestop()tears downunsubscribe('sub-1')aftershutdown()Column ② is not decoration: pinning only ① would go green on an implementation that
simply deletes
stop()from all six, which breaks the embedders the alias exists for.Each member also pins that a teardown on a never-started plugin is a no-op rather than a
throw — the kernel calls
destroy()on every plugin it walks.4. Ablation — predicted before mutating, matched exactly
Mutation:
git checkout origin/main --over the six plugin sources, tests and changesetkept, no rebuild. Prediction written down first; the measured result matched it in
every one of the 17 cases.
connector-rest1 failed | 1 passed (2)—expected [ 'rest' ] to not include 'rest'connector-slack1 failed | 1 passed (2)—expected [ 'slack' ] to not include 'slack'connector-openapi1 failed | 1 passed (2)—expected [ 'mini' ] to not include 'mini'service-knowledge3 failed (3)—expected [] to deeply equal [ 'sub-1' ],plugin.destroy is not a functionplugin-approvals3 failed (3)—expected [] to include 'approvals-sla-escalation',plugin.destroy is not a functionplugin-reports3 failed | 2 passed (5)—expected [] to include 'reports.dispatch',plugin.destroy is not a functionThe passing cells are the informative half and were predicted as passes: the three
connectors' pre-repair
stop(_ctx)ignored its context, andplugin-reports' usedctxonly inside a failure catch, so calling the alias directly worked before the repair too —
which is exactly the compatibility the alias preserves.
plugin-approvalsandservice-knowledgeresolved services throughctxat teardown, so their alias legs gored when called with no argument. Direction: straight red, not "more diagnostics" and
not inverted.
Resolution, proved rather than asserted. At the moment the ablation ran, the six
subject packages had no
dist/at all in this worktree — only their dependencyclosures had been built — so a test could not have been reading a built artifact of the
subject even in principle; and the ablation touched only
src/with no rebuild, so thered/green flip is itself the positive control. (
service-knowledgewas built afterwards,for
check:type-check-debt, which refuses to measure against an unbuilt closure.) Theirdependencies (
@objectstack/core,@objectstack/objectql,@objectstack/driver-sql,@objectstack/service-automation) do resolve throughexports→dist/and were builtfirst — except in
service-knowledge, whose ownvitest.config.tsaliases@objectstack/coreto source, and inconnector-openapivia the new config describedbelow.
Restore:
git checkout HEAD --over the same six, thengit hash-objecton each filematched the pre-ablation hash byte for byte, and
git statuscame back clean.Out of the declared file surface
packages/connectors/connector-openapi/vitest.config.tsis new, and it is notcosmetic:
pnpm check:test-source-aliaswent red the moment the new test imported@objectstack/coreand@objectstack/service-automationinto a package whoseKNOWN_UNALIASED_TEST_IMPORTSentry is['@objectstack/spec']alone. That registry isshrink-only and the gate's own instruction is to alias, not to widen. The config carries
only the two anchored entries the gate named and no
testblock, so the package'sthree existing test files keep running on vitest's defaults.
Verification
Suites — every affected package's full suite, not just the new file:
@objectstack/plugin-reportsTest Files 4 passed (4)·Tests 75 passed (75)@objectstack/connector-restTest Files 4 passed (4)·Tests 18 passed (18)@objectstack/connector-slackTest Files 3 passed (3)·Tests 10 passed (10)@objectstack/connector-openapiTest Files 4 passed (4)·Tests 34 passed (34)@objectstack/plugin-approvalsTest Files 27 passed (27)·Tests 522 passed (522)@objectstack/service-knowledgeTest Files 4 passed (4)·Tests 40 passed (40)pnpm typecheckover the five packages that declare the script — allDone,TYPECHECK_EXIT=0.@objectstack/service-knowledgedeclares notypecheckscript (itcarries a ledger entry instead), so a
--filterrun for it would be a zero-match exit 0;it is named here rather than counted as a pass — its tsc program was measured directly
instead (see the
check:type-check-debtnote below).Gates, quoting each one's own verdict line — union re-derived by
node scripts/pm/dispatch-gates.mjswith no path arguments, on a clean tree, atc846181d3— and re-run in full after themainmerge, see the section below:check:test-source-aliascheck-test-source-alias OK — 72 packages with tests scanned; 61 registered as still resolving a workspace dep through dist/check:type-source-resolutioncheck-type-source-resolution OK — 76 packages with a tsconfig.json scannedcheck:slot-lookup✓ slot-lookup ratchet holds: 107 unswept site(s) in 25 file(s), none newcheck:changeset-gate-self-tests✓ check-empty-changeset --self-test: 118 assertions …(all three self-tests pass)check:objectui-changeset✓ objectui-range --self-test: all checks passedcheck-empty-changeset.mjs✓ No empty-frontmatter changeset introduced by this diff (1 declaring changeset(s) added)check-changeset-no-major.mjs✓ This diff introduces no major bumpcheck-adr-0087-registration.mjs✓ this PR adds no declared-breaking changeset (1 non-breaking changeset(s) seen)docs-audit/check-affected-docs.mjscheck:query-options-erasure✓ query-options-erasure ratchet holds: 67 unswept non-test site(s) in 17 file(s), none newcheck:type-check-coveragecheck-type-check-coverage: OK — 64/77 workspace packages type-checkedcheck:type-check-debtcheck-type-check-coverage --re-measure: OK — 33 ledger entr(ies) re-measured in 385.1s, 1912 raw tsc error(s) total, none above its recorded numbercheck:i18ncheck-i18n-bundles: OK (9 package(s) — all bundles in sync, no undeclared authoring keys)check:engine-double-contractcheck-engine-double-contract: OK — 371 pinned, 133 in the DEBT ledger, 2 exemptcheck:where-matcher✓ where-matcher conformance holds: 271 matcher(s) discovered … none newcheck:route-envelope✓ Express-style response modules — 4 module(s) discovered and audited(--self-testalso passes)check:dispatcher-error-vocabularycheck-dispatcher-error-vocabulary: OK — 21 unregistered code-stamping site(s), all classified(--self-testalso passes)The last two are the class #10309 record: the derived union did not name either of
them on this change set, on either the pre-commit or the post-commit derivation. They were
run explicitly anyway, self-test and real run, and both are green.
check:type-check-debtalso reports, informationally, that@objectstack/plugin-auth's TEST_DEBT records 109 while tsc now measures 97. That gap is#10615 and is not touched here — no
--lowerwas run.Four gates went red during development and are green now, all four genuinely this PR's:
check:slot-lookup— an untypedgetService('objectql')in the approvals test.check:test-source-alias— the connector-openapi config described above.pnpm typecheck(plugin-reports) —logLevelis not a member ofObjectKernelConfig;both kernels take an optional partial
LoggerConfigunder aloggerkey, and anas anywas hiding it.check:type-check-debt— the knowledge pin's relative import had no.jsextension,a TS2835 under NodeNext inside a package whose tsc program includes
src/__tests__.@objectstack/service-knowledgecarries a frozen 10-error entry; the new error took itto 11 against a shrink-only ratchet. Measured back at exactly 10 after the fix. Copying
the neighbouring test file's extension-less spelling — itself one of that package's two
ledgered TS2835s — was the mistake.
Two things deliberately NOT done here
Pluginthat declaresstop()/shutdown()/close()with nodestroy()— is guard: fail anyPluginimplementation that declaresstop()(orshutdown()/close()) with nodestroy()— the kernel only ever callsdestroy()#10619, devx-lane gate work. Not built here, andpackages/coreis untouched. One note for that card from doing these six by hand: thecriterion cannot be "declares
stop()and nodestroy()", because after this PR allsix declare both and are correct. It has to be reachability — a teardown-shaped
method that no kernel path calls and that does not delegate to
destroy().ReportsServicePlugin's documentedsetIntervalfallback is unreachable onObjectKernel, becausepreInjectCoreFallbacks()always supplies ajobservice —and
createMemoryJob()'sschedule()owns no timer and never fires. So on a defaultObjectKernelstack withoutservice-job, scheduled reports never dispatch at all,while the log line reads like success. That is why the timer leg of the reports pin
boots a
LiteKernel; the reason is written down atbootReportsLiteKernel.Docs-drift flags: three pages, no staleness — the reading and its control
The docs-drift bot flagged three hand-written pages because they name symbols this PR
touches. Re-measured here rather than taken on trust:
stop()/.stop/destroy()/teardown/shutdown)content/docs/ai/knowledge-rag.mdxcontent/docs/getting-started/your-first-project.mdxcontent/docs/protocol/knowledge.mdxThe control is what makes the zero worth anything: the same grep over the same three files
DOES find
KnowledgeServicePlugin/ConnectorOpenApiPlugin/ConnectorRestPlugin, sothe files exist, are readable, and really do name the touched symbols — a real zero, not a
failed search. These pages name the plugin classes and say nothing about which lifecycle
hook their teardown lives on, which is the only thing this PR moves. The bot listed them
correctly (it is precision-first on touched symbols); the answer is simply "no staleness".
content/docs/releases/v16.mdxwas flagged too and is release-owned and read-only — nottouched. This PR changes 0 files under
content/docs/.Merged
main, and the #10619 gate's baselinescripts/check-plugin-teardown-shape.mjs— the #10619 gate — landed onmainat699132f25, after this branch was cut. ItsKNOWN_TEARDOWN_UNREACHEDlist isshrink-only and a stale entry is itself a failure, and all six plugins repaired here
were on it. Merging this PR without touching that list would have reddened
check:plugin-teardown-shapeonmainfor everyone — or, more likely, been evicted fromthe merge queue and rebuilt the queue for every PR behind it. That is the #9371
blast-radius shape, on a PR whose subject is #9371's defect class.
So
origin/mainis merged in (merge commit, never a rebase or a force-push) and exactlythose six entries are deleted. Which six was taken from the gate's own output, not from
line numbers: run before the edit, it named precisely these six as
no longer unreached,and each was independently confirmed to declare a real
destroy()in this tree.The other five entries are untouched —
MetadataPlugin,AppPluginandExternalValidationPlugin(alias spelled as an arrow property) andEmailServicePlugin/WebhookOutboxPlugin(spelleddispose). None has adestroy()yet; they are #10772'scard, and deleting them would be a false declaration that they are fixed.
--self-test✓ check-plugin-teardown-shape self-test: 47 cases pass (real pre-#10375 fixture reds, the repaired file and both delegating-alias directions stay green, every roster name reds, every excluded name stays green, and all five refusals are paired with a tree that still returns a verdict).exit 0✓ check:plugin-teardown-shape: 57 Plugin implementation(s) across 4387 source(s) under packages/**; every teardown-shaped method (stop / shutdown / close / dispose) sits beside a real destroy() (5 known-unreached, ⛔ SHRINK-ONLY, repair tracked on #10371).exit 0Held count observed: 5.
--listnames them, and none is one of this card's six.repair: '#10371', and the gate's verdict line therefore says "repair tracked on#10371". Once this PR merges and #10371 closes, that attribution points at a closed card.
The five belong to #10772. Correcting the field means editing another card's entries, so
it is reported rather than done.
Everything re-verified on the merged tree
All six suites, re-run after the merge, unchanged: plugin-reports 75/75, connector-rest
18/18, connector-slack 10/10, connector-openapi 34/34, plugin-approvals 522/522,
service-knowledge 40/40.
typecheckDoneon all five packages that declare it,TYPECHECK_EXIT=0. The gate union was re-derived atd06b45700against the new mergebase
699132f25: 128 families now, and it names three gates it did not name before —check:cross-package-test-inputs(OK: 13 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob, exit 0) andcheck-plugin-teardown-shape.mjsitself, now that this PR edits it. Every previouslyquoted gate was re-run on the merged tree and is still green, with the ratchets now
reconciling against
699132frather than53a48c9.One gate NOT re-run on the merged tree, and what stands in for it
check:type-check-debt --re-measurerefused on the merged worktree —--re-measure cannot run: 33 workspace dependenc(ies) of the ledgered packages have no built type entry point on disk— because the worktree was recreated for the merge and lost everydist/.That refusal is not measured, and it is reported as such rather than as a pass. It was
green at
c846181d3before the merge.What stands in for it is targeted rather than hopeful, because this PR can only move the
ledger through its own files. Four of the six packages carry entries —
connector-openapi(TEST_DEBT 5),
connector-rest(TEST_DEBT 1),plugin-approvals(TEST_DEBT 348),service-knowledge(DEBT 10) — and the other two carry none. Each was measured directlywith
tscover the same program the ledger describes:connector-openapiconnector-restplugin-approvalsservice-knowledgeAll four reproduce their recorded number exactly, which is the positive control that the
stand-in program is the right one, and none of this PR's new test files contributes a
single error. The three TEST_DEBT packages are the ones that matter here: their tsconfigs
exclude
**/*.test.ts, so a new test file is invisible topnpm typecheckand visibleonly to the re-measure — which is exactly how a green local run could have hidden a red
CI gate.
Generated by Claude Code