From 9ba8f3ab12e0eb11d5b77cba872be30a1d5754ff Mon Sep 17 00:00:00 2001 From: Warren Buffett Date: Wed, 26 Aug 2026 12:46:02 +0000 Subject: [PATCH 1/2] fix(spec,core): PluginHealthMonitor stops claiming a restart it never performed; the three PluginHealthCheck restart keys retired (#12032, ADR-0049) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `attemptRestart` called `plugin.destroy()` and stopped there. The comment above the call read "Call destroy and init to restart", and `init` appeared in `health-monitor.ts` ONLY inside that comment. A plugin that crossed `failureThreshold` with `autoRestart: true` got destroy, a log line reading 'Plugin restarted', status `recovering`, and periodic checks that carried on against the destroyed instance — which the default `plugin-loaded` check passes forever, so the terminal report on a torn-down plugin was `healthy`. ENFORCE was unavailable: `Plugin.init(ctx)` needs a `PluginContext`, and the only two `plugin.init(...)` call sites are the kernel's own boot loops with a context that is private on ObjectKernel and protected on KernelBase, so a host-provided re-init hook would have had nothing to call. EXPERIMENTAL needs a roadmap and the `docs/` corpus has zero mentions of plugin auto-restart against 118 control hits. So the declaration goes: `autoRestart`, `maxRestartAttempts` and `restartBackoff` are tombstoned, and the monitor no longer destroys anything. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01W6HFzyH98W1YaQXhJUJt6o --- ...plugin-auto-restart-never-reinitialised.md | 94 +++++ content/docs/protocol/kernel/lifecycle.mdx | 26 +- .../kernel/plugin-lifecycle-advanced.mdx | 6 +- packages/core/examples/phase2-integration.ts | 7 +- packages/core/src/health-monitor.test.ts | 359 ++++++++++++------ packages/core/src/health-monitor.ts | 250 +++++++----- packages/spec/authorable-defaults/kernel.json | 3 - packages/spec/authorable-surface/kernel.json | 6 +- .../kernel/plugin-lifecycle-advanced.test.ts | 70 +++- .../kernel/plugin-lifecycle-advanced.zod.ts | 152 +++++++- ....kernel__PluginHealthCheck__autoRestart.ts | 55 +++ ...__PluginHealthCheck__maxRestartAttempts.ts | 55 +++ ...rnel__PluginHealthCheck__restartBackoff.ts | 55 +++ ...plugin-auto-restart-never-reinitialised.ts | 92 +++++ packages/spec/src/migrations/registry.ts | 247 ++++++++++++ 15 files changed, 1217 insertions(+), 260 deletions(-) create mode 100644 .changeset/plugin-auto-restart-never-reinitialised.md create mode 100644 packages/spec/src/migrations/entries/retired-keys/18.kernel__PluginHealthCheck__autoRestart.ts create mode 100644 packages/spec/src/migrations/entries/retired-keys/18.kernel__PluginHealthCheck__maxRestartAttempts.ts create mode 100644 packages/spec/src/migrations/entries/retired-keys/18.kernel__PluginHealthCheck__restartBackoff.ts create mode 100644 packages/spec/src/migrations/entries/semantic/18.plugin-auto-restart-never-reinitialised.ts diff --git a/.changeset/plugin-auto-restart-never-reinitialised.md b/.changeset/plugin-auto-restart-never-reinitialised.md new file mode 100644 index 0000000000..9b9dbe6125 --- /dev/null +++ b/.changeset/plugin-auto-restart-never-reinitialised.md @@ -0,0 +1,94 @@ +--- +"@objectstack/spec": minor +"@objectstack/core": minor +--- + +fix(spec,core): `PluginHealthMonitor` stops claiming a restart it never performed; the three `PluginHealthCheck` restart keys retired (#12032, ADR-0049) + + + +**BREAKING** accept-set narrowing, landing after the v17.0.0 cut (the lockstep +launch-window convention ships it as `minor`; the prescriptions are registered +under protocol major 18 — three `RETIRED_KEYS_BY_MAJOR[18]` entries plus the D3 +semantic entry `plugin-auto-restart-never-reinitialised` — where +`os migrate meta` users will look). Graded `minor` rather than `major` for the +same reason #12340 and #12428 were, the day before, in this same module. + +## What was measured + +`PluginHealthMonitor.attemptRestart` called `plugin.destroy()` and stopped +there. The comment above the call read *"Call destroy and init to restart"*, +and `init` appeared in `health-monitor.ts` **only inside that comment**. So a +plugin whose health checks crossed `failureThreshold` with `autoRestart: true` +got: `destroy()`, a log line reading `Plugin restarted`, status `recovering`, +and periodic health checks that carried on running against the destroyed +instance. The default check when no `checkMethod` resolves is +`{ name: 'plugin-loaded', status: 'passed' }`, which a destroyed object passes +indefinitely — so the **terminal** report on a torn-down, never-re-initialised +plugin was `healthy`. + +Reproduced at `ee3595cefd` before anything was changed, with +`successThreshold: 3`: + +``` +round 1 (failing): status=failed destroyed=0 alive=true +after backoff: status=recovering destroyed=1 alive=false +recovery round 1: status=recovering destroyed=1 alive=false +recovery round 2: status=recovering destroyed=1 alive=false +recovery round 3: status=healthy destroyed=1 alive=false +``` + +#11955 made that report *more* convincing rather than less: reaching `healthy` +now costs `successThreshold` consecutive passing rounds, so a destroyed plugin +has to earn a declared number of passes before it is misreported. +`restartAttempts` was incremented as though a restart had occurred, and +`maxRestartAttempts` / `restartBackoff` scheduled further "restarts" of a plugin +that was never brought back up. + +## Why REMOVE and not the other two ADR-0049 states + +**ENFORCE** would have to build the restart, and the class cannot host one. +`Plugin.init(ctx)` needs a `PluginContext`; the only two `plugin.init(...)` call +sites in the tree are the kernel's own boot loops (`kernel-base.ts:202`, +`kernel.ts:607`), both over the full plugin list, with a context that is +`private` on `ObjectKernel` and `protected` on `KernelBase`. No host can obtain +one, so a host-provided re-init hook would have had nothing to call. (Positive +control for that scan: the same pass resolves five real non-test +`plugin.destroy()` call sites, so it does see lifecycle drivers.) Building a +per-plugin re-init API for a caller that does not exist — no runtime constructs +`PluginHealthMonitor` (#11825) — is the speculation ADR-0049's staged decision +names as the wrong default at this milestone, where the shippable liability is +the false promise and not the missing feature. + +**EXPERIMENTAL** requires a roadmap. A scan of the whole `docs/` planning + ADR +corpus returned **zero** mentions of plugin auto-restart, against 118 control +hits for "health" and 13 for "hot reload" in the same corpus. + +`maxRestartAttempts` and `restartBackoff` leave with `autoRestart` rather than +as a tidy-up: with no restart, *"Maximum restart attempts before giving up"* and +*"Backoff strategy for restart delays"* have nothing left to be the vocabulary +**of** — the test that took `distributedConfig` out with the `stateStrategy` +value it was documented as requiring (#12340). + +## What changes for a host + +All three keys are **tombstoned**, not deleted: `PluginHealthCheckSchema` is not +`.strict()`, so a bare deletion would be a silent strip (#3733, ADR-0104) — a +milder form of the defect being retired. A TypeScript host gets a `tsc` error +(the keys are typed `never`); a parse raises the prescription; and +`PluginHealthMonitor.registerPlugin` refuses a hand-built config carrying any of +them with an ADR-0112 envelope (`code: VALIDATION_ERROR`, `status: 400`), thrown +before any state is stored so a refused config leaves no half-registered plugin +behind. + +`PluginHealthMonitor` no longer calls `plugin.destroy()` at all. A plugin that +crosses `failureThreshold` is reported `degraded` / `unhealthy` / `failed` and +left running; acting on that is the host's job in this host-driven library +(#11825 route 2). Poll `getHealthStatus(pluginName)` / `getHealthReport(pluginName)` +and restart at the level that owns the plugin's lifetime. + +Everything else in the monitor is unchanged: registration, periodic checks, the +`timeout` race and its refd-timer guard (#4875), both failure routes sharing the +counters (#11852), and `successThreshold` binding from every status that records +a failure (#11955). `recovering` is now written only by the success branch — +the one writer that ever meant it. diff --git a/content/docs/protocol/kernel/lifecycle.mdx b/content/docs/protocol/kernel/lifecycle.mdx index 11aa2e447e..ff3e155f8a 100644 --- a/content/docs/protocol/kernel/lifecycle.mdx +++ b/content/docs/protocol/kernel/lifecycle.mdx @@ -693,8 +693,7 @@ export const salesforcePlugin = { const monitor = new PluginHealthMonitor(kernel.logger); // `registerPlugin` takes the PARSED config, so parse it: the schema fills in -// interval 30000, timeout 5000, failureThreshold 3, successThreshold 1, -// autoRestart false, maxRestartAttempts 3, restartBackoff 'exponential'. +// interval 30000, timeout 5000, failureThreshold 3 and successThreshold 1. monitor.registerPlugin( salesforcePlugin.name, PluginHealthCheckSchema.parse({ checkMethod: 'healthCheck' }), @@ -734,9 +733,26 @@ failure resets the success count to zero — both routes included — so a throw part-way through a recovery starts the next attempt at one rather than resuming where it left off. The symmetry holds the other way too: a passing round resets the failure count, so `failureThreshold` likewise counts only an unbroken run. -A successful auto-restart lands the plugin in `recovering` with **both** -counters cleared, so a restarted plugin still owes a full `successThreshold` of -passing rounds before it reads `healthy`. + +### The monitor reports; it does not act + +Nothing above does anything **to** the plugin. A failing plugin is labelled +`degraded`, `unhealthy` or `failed` and left running; the monitor never calls +`destroy()`, and acting on what it reports is the host's job — this is a +host-driven library, and the host is the only party that owns the plugin's +lifetime. + +It used to claim otherwise. `PluginHealthCheck` carried `autoRestart`, +`maxRestartAttempts` and `restartBackoff`, and a plugin that crossed +`failureThreshold` with `autoRestart: true` got `plugin.destroy()` called on it +— and nothing else. `init()` was never called, because the monitor has no +`PluginContext` to call it with and no way to obtain one. The plugin was then +logged as `Plugin restarted`, marked `recovering`, and kept under periodic +checks it went on passing from the grave: the default check when no +`checkMethod` resolves is `plugin-loaded`, which a destroyed object satisfies +forever. So the terminal report on a torn-down plugin was `healthy`. The three +keys were removed in `@objectstack/spec` 18 under ADR-0049 enforce-or-remove; +`PluginHealthMonitor.registerPlugin` refuses a config that still carries one. At the default `successThreshold: 1` none of this is observable: the first passing round satisfies the count from every status, and `recovering` is never diff --git a/content/docs/references/kernel/plugin-lifecycle-advanced.mdx b/content/docs/references/kernel/plugin-lifecycle-advanced.mdx index 00fcf4245d..4263d7df98 100644 --- a/content/docs/references/kernel/plugin-lifecycle-advanced.mdx +++ b/content/docs/references/kernel/plugin-lifecycle-advanced.mdx @@ -63,9 +63,9 @@ const result = HotReloadConfigSchema.parse(data); | **failureThreshold** | `integer` | optional (default: `3`) | Consecutive failures needed to mark unhealthy | | **successThreshold** | `integer` | optional (default: `1`) | Consecutive successes needed to mark healthy | | **checkMethod** | `string` | optional | Method name to call for health check | -| **autoRestart** | `boolean` | optional (default: `false`) | Automatically restart plugin on health check failure | -| **maxRestartAttempts** | `integer` | optional (default: `3`) | Maximum restart attempts before giving up | -| **restartBackoff** | `Enum<'fixed' \| 'linear' \| 'exponential'>` | optional (default: `"exponential"`) | Backoff strategy for restart delays | +| **autoRestart** | `never` | optional | [REMOVED] `PluginHealthCheck.autoRestart` was removed in @objectstack/spec 18 (#12032, ADR-0049 enforce-or-remove) — it never restarted a plugin. A `PluginHealthMonitor` never restarted anything. `attemptRestart` called `plugin.destroy()` and stopped there — the in-source comment said "Call destroy and init to restart", but `init` appeared in `health-monitor.ts` ONLY inside that comment. What a plugin actually got was: destroy, a log line reading 'Plugin restarted', status `recovering`, and periodic health checks continuing against the destroyed instance — which the default check (`{ name: 'plugin-loaded', status: 'passed' }`, used whenever no `checkMethod` resolves) passes forever, so the terminal report on a destroyed, never-re-initialised plugin was `healthy`. Delete the key. Restarting a plugin is the HOST's job in this host-driven library, and the monitor could not do it even in principle: `Plugin.init(ctx)` needs a `PluginContext`, which only the kernel constructs and which it exposes to nobody (`ObjectKernel.context` is private; `KernelBase.createContext` is protected). Poll `getHealthStatus(pluginName)` / `getHealthReport(pluginName)` and act on `unhealthy` / `failed` at the level that owns the plugin's lifetime — recreate the kernel, or let your supervisor restart the process. The monitor reports; it does not act. | +| **maxRestartAttempts** | `never` | optional | [REMOVED] `PluginHealthCheck.maxRestartAttempts` was removed in @objectstack/spec 18 (#12032, ADR-0049 enforce-or-remove) — it capped a restart that never happened. A `PluginHealthMonitor` never restarted anything. `attemptRestart` called `plugin.destroy()` and stopped there — the in-source comment said "Call destroy and init to restart", but `init` appeared in `health-monitor.ts` ONLY inside that comment. What a plugin actually got was: destroy, a log line reading 'Plugin restarted', status `recovering`, and periodic health checks continuing against the destroyed instance — which the default check (`{ name: 'plugin-loaded', status: 'passed' }`, used whenever no `checkMethod` resolves) passes forever, so the terminal report on a destroyed, never-re-initialised plugin was `healthy`. The cap counted destroy calls, so raising it only scheduled further "restarts" of a plugin that was never brought back up. Delete the key. Restarting a plugin is the HOST's job in this host-driven library, and the monitor could not do it even in principle: `Plugin.init(ctx)` needs a `PluginContext`, which only the kernel constructs and which it exposes to nobody (`ObjectKernel.context` is private; `KernelBase.createContext` is protected). Poll `getHealthStatus(pluginName)` / `getHealthReport(pluginName)` and act on `unhealthy` / `failed` at the level that owns the plugin's lifetime — recreate the kernel, or let your supervisor restart the process. The monitor reports; it does not act. | +| **restartBackoff** | `never` | optional | [REMOVED] `PluginHealthCheck.restartBackoff` was removed in @objectstack/spec 18 (#12032, ADR-0049 enforce-or-remove) — it delayed a restart that never happened. A `PluginHealthMonitor` never restarted anything. `attemptRestart` called `plugin.destroy()` and stopped there — the in-source comment said "Call destroy and init to restart", but `init` appeared in `health-monitor.ts` ONLY inside that comment. What a plugin actually got was: destroy, a log line reading 'Plugin restarted', status `recovering`, and periodic health checks continuing against the destroyed instance — which the default check (`{ name: 'plugin-loaded', status: 'passed' }`, used whenever no `checkMethod` resolves) passes forever, so the terminal report on a destroyed, never-re-initialised plugin was `healthy`. The chosen strategy only moved when the destroy landed. Delete the key. Restarting a plugin is the HOST's job in this host-driven library, and the monitor could not do it even in principle: `Plugin.init(ctx)` needs a `PluginContext`, which only the kernel constructs and which it exposes to nobody (`ObjectKernel.context` is private; `KernelBase.createContext` is protected). Poll `getHealthStatus(pluginName)` / `getHealthReport(pluginName)` and act on `unhealthy` / `failed` at the level that owns the plugin's lifetime — recreate the kernel, or let your supervisor restart the process. The monitor reports; it does not act. | --- diff --git a/packages/core/examples/phase2-integration.ts b/packages/core/examples/phase2-integration.ts index e270a0691d..f5657b6e9d 100644 --- a/packages/core/examples/phase2-integration.ts +++ b/packages/core/examples/phase2-integration.ts @@ -270,9 +270,10 @@ async function example() { timeout: 5000, failureThreshold: 3, successThreshold: 1, - autoRestart: true, - maxRestartAttempts: 3, - restartBackoff: 'exponential', + // [#12032] `autoRestart` / `maxRestartAttempts` / `restartBackoff` + // removed: the monitor never restarted anything (it called + // `plugin.destroy()` and reported the corpse `healthy`), so the keys + // were retired under ADR-0049. Act on `getHealthStatus()` in the host. }, // Hot reload diff --git a/packages/core/src/health-monitor.test.ts b/packages/core/src/health-monitor.test.ts index f32e836f27..f65b6504a9 100644 --- a/packages/core/src/health-monitor.test.ts +++ b/packages/core/src/health-monitor.test.ts @@ -24,9 +24,6 @@ describe('PluginHealthMonitor', () => { timeout: 1000, failureThreshold: 3, successThreshold: 1, - autoRestart: false, - maxRestartAttempts: 3, - restartBackoff: 'exponential', }; monitor.registerPlugin('test-plugin', config); @@ -39,9 +36,6 @@ describe('PluginHealthMonitor', () => { timeout: 1000, failureThreshold: 3, successThreshold: 1, - autoRestart: false, - maxRestartAttempts: 3, - restartBackoff: 'fixed', }; monitor.registerPlugin('test-plugin', config); @@ -54,9 +48,6 @@ describe('PluginHealthMonitor', () => { timeout: 1000, failureThreshold: 3, successThreshold: 1, - autoRestart: false, - maxRestartAttempts: 3, - restartBackoff: 'linear', }; monitor.registerPlugin('plugin1', config); @@ -74,9 +65,6 @@ describe('PluginHealthMonitor', () => { timeout: 1000, failureThreshold: 3, successThreshold: 1, - autoRestart: false, - maxRestartAttempts: 3, - restartBackoff: 'exponential', }; monitor.registerPlugin('test-plugin', config); @@ -102,9 +90,6 @@ describe('PluginHealthMonitor', () => { timeout: 120_000, failureThreshold: 3, successThreshold: 1, - autoRestart: false, - maxRestartAttempts: 3, - restartBackoff: 'fixed', checkMethod: 'healthCheck', ...overrides, }); @@ -247,50 +232,68 @@ describe('PluginHealthMonitor', () => { }); }); - // #11852 — `autoRestart` covers BOTH failure routes, not only the milder one. + // ── #12032 — the monitor REPORTS; it never destroys, and never lies ─────── // - // `performHealthCheck` fails two disjoint ways: the check RETURNS a failure - // (`false` / `{ status: 'unhealthy' }`), or it THROWS — which, because + // This block replaces #11852's `autoRestart covers both failure routes`. + // Those five tests are REWRITTEN here rather than deleted, and the change is + // declared because four of them pinned the false contract as the contract: + // each drove a plugin past `failureThreshold` and then asserted + // `destroyed.count === 1` and status `recovering` — i.e. asserted that a + // "restart" had happened. It had not. `attemptRestart` called + // `plugin.destroy()` and stopped; `init` appeared in `health-monitor.ts` + // ONLY inside the comment that claimed both were called. The assertions were + // true readings of a false behaviour. + // + // What #11852 actually established SURVIVES and is re-pinned below: both + // failure routes — the check RETURNS a failure, or it THROWS (which, because // `raceCheckTimeout` rejects rather than resolving, includes every `timeout` - // overrun. Only the returned route ever reached `config.autoRestart`, so a - // plugin that threw or hung until its timeout — the severer failure of the - // two — was marked `failed` and never restarted, whatever the config said. + // overrun) — share the counters, and a throw keeps its own `failed` label. + // What leaves is the restart decision they were also said to share, because + // there is no restart (#12032: `autoRestart` / `maxRestartAttempts` / + // `restartBackoff` retired in @objectstack/spec 18, ADR-0049). // - // These pin the observable consequence, never the source: `attemptRestart` - // is the only caller of `plugin.destroy()` and the only writer of - // `recovering`, so those two readings together mean a restart happened and - // nothing else can produce them. Asserting that some shared helper was - // called would be a tautology any refactor could satisfy. - describe('autoRestart covers both failure routes (#11852)', () => { - /** `calculateBackoff(0, 'fixed')` — the delay before the first restart. */ - const FIRST_RESTART_BACKOFF_MS = 1_000; - - const restartConfig = ( + // These pin the observable consequence, never the source. `plugin.destroy()` + // had exactly one caller in this class, so a `destroyed.count` of zero is + // the whole claim "the monitor does not tear plugins down"; asserting that + // some method is absent would be a tautology any refactor could satisfy, and + // asserting "`init` is called" would be satisfied by a no-op `init`. + describe('a failing plugin is reported, never destroyed (#12032)', () => { + const INTERVAL_MS = 10_000; + /** The backoff the retired restart used to wait out before destroying. */ + const FORMER_RESTART_BACKOFF_MS = 1_000; + + const failingConfig = ( overrides: Partial = {} ): PluginHealthCheckParsed => ({ - interval: 10_000, + interval: INTERVAL_MS, timeout: 100, failureThreshold: 2, successThreshold: 1, - autoRestart: true, - maxRestartAttempts: 3, - restartBackoff: 'fixed', checkMethod: 'healthCheck', ...overrides, }); - const restartable = (name: string, healthCheck: () => unknown) => { + /** + * A plugin that records whether it is still ALIVE — not merely whether + * `destroy` was called. `alive` is what an operator's `healthy` reading is + * supposed to be about, and it is the reading the old behaviour got wrong. + */ + const observable = (name: string, healthCheck: () => unknown) => { const destroyed = { count: 0 }; + const state = { alive: true }; const plugin = { name, version: '1.0.0', - init: () => {}, + init: () => { + state.alive = true; + }, destroy: async () => { destroyed.count++; + state.alive = false; }, healthCheck, } as unknown as Plugin; - return { plugin, destroyed }; + return { plugin, destroyed, state }; }; beforeEach(() => { @@ -301,38 +304,97 @@ describe('PluginHealthMonitor', () => { vi.useRealTimers(); }); - it('restarts a plugin whose check THROWS, once failureThreshold accumulates', async () => { - const config = restartConfig(); - const { plugin, destroyed } = restartable('throwing-plugin', async () => { + // ⭐ The terminal-state pin. Not "init is called" — that is green the + // moment someone adds a no-op `init`. This asserts what an operator READS + // at the end of the sequence, and the invariant behind it: a plugin that + // reads `healthy` is a plugin that is alive. + // + // The sequence is the one that used to produce the false report: drive a + // plugin past `failureThreshold` (the point the retired `autoRestart` + // fired), wait out the former restart backoff, then feed it + // `successThreshold` CONSECUTIVE passing rounds — the #11955 gate — and + // read the status an operator would read. + // + // Before #12032 this same drive ended at: + // after backoff: status=recovering destroyed=1 alive=false + // recovery round 3: status=healthy destroyed=1 alive=false + it('never reports `healthy` for a plugin it has torn down', async () => { + const config = failingConfig({ failureThreshold: 1, successThreshold: 3 }); + const mode = { current: 'throw' as 'throw' | 'pass' }; + const { plugin, destroyed, state } = observable('observed-plugin', async () => { + if (mode.current === 'throw') throw new Error('check exploded'); + return true; + }); + + monitor.registerPlugin('observed-plugin', config); + monitor.startMonitoring('observed-plugin', plugin); + + // The failing round that used to trigger the "restart". + await vi.advanceTimersByTimeAsync(0); + expect(monitor.getHealthStatus('observed-plugin')).toBe('failed'); + expect(destroyed.count).toBe(0); + expect(state.alive).toBe(true); + + // The window the destroy used to land in. + await vi.advanceTimersByTimeAsync(FORMER_RESTART_BACKOFF_MS); + expect(destroyed.count, 'the monitor must not tear the plugin down').toBe(0); + expect(monitor.getHealthStatus('observed-plugin')).toBe('failed'); + + // `successThreshold` consecutive successes — the walk that used to end + // at `healthy` on a destroyed instance. + mode.current = 'pass'; + for (let round = 1; round <= config.successThreshold; round++) { + await vi.advanceTimersByTimeAsync(INTERVAL_MS); + await vi.advanceTimersByTimeAsync(0); + // THE INVARIANT, checked at every step and not only at the end: a + // `healthy` reading is only ever about a plugin that is alive. + if (monitor.getHealthStatus('observed-plugin') === 'healthy') { + expect(state.alive, '`healthy` was reported for a destroyed plugin').toBe(true); + } + } + + expect(monitor.getHealthStatus('observed-plugin')).toBe('healthy'); + expect(state.alive, 'the plugin that reads healthy must actually be alive').toBe(true); + expect(destroyed.count).toBe(0); + + monitor.stopMonitoring('observed-plugin'); + }); + + it('leaves a plugin whose check THROWS alone, however many rounds fail', async () => { + // Was: 'restarts a plugin whose check THROWS, once failureThreshold + // accumulates', asserting destroyed.count === 1 and `recovering`. + const config = failingConfig(); + const { plugin, destroyed, state } = observable('throwing-plugin', async () => { throw new Error('check exploded'); }); monitor.registerPlugin('throwing-plugin', config); monitor.startMonitoring('throwing-plugin', plugin); - // Round 1 (the immediate initial check) is below `failureThreshold`. await vi.advanceTimersByTimeAsync(0); expect(monitor.getHealthStatus('throwing-plugin')).toBe('failed'); - expect(destroyed.count).toBe(0); - // Round 2 reaches the threshold and arms the restart's backoff. + // Past `failureThreshold`, and past the former backoff window, twice + // over: the retired path would have destroyed by now and moved the + // status to `recovering`. await vi.advanceTimersByTimeAsync(config.interval); - await vi.advanceTimersByTimeAsync(0); - // Not yet: the restart waits out `restartBackoff` first. Without this the - // reading below could be "restarted eventually" rather than "restarted". - expect(destroyed.count).toBe(0); + await vi.advanceTimersByTimeAsync(FORMER_RESTART_BACKOFF_MS); + await vi.advanceTimersByTimeAsync(config.interval); + await vi.advanceTimersByTimeAsync(FORMER_RESTART_BACKOFF_MS); - await vi.advanceTimersByTimeAsync(FIRST_RESTART_BACKOFF_MS); - expect(destroyed.count).toBe(1); - expect(monitor.getHealthStatus('throwing-plugin')).toBe('recovering'); + expect(destroyed.count).toBe(0); + expect(state.alive).toBe(true); + expect(monitor.getHealthStatus('throwing-plugin')).toBe('failed'); monitor.stopMonitoring('throwing-plugin'); }); - it('restarts a plugin whose check exceeds `timeout` — the severest route', async () => { - const config = restartConfig(); - // Never settles: the timeout guard is what ends every round. - const { plugin, destroyed } = restartable( + it('leaves a plugin that exceeds `timeout` alone — and still reports the timeout', async () => { + // Was: 'restarts a plugin whose check exceeds `timeout` — the severest + // route'. The REPORT half of that test is kept verbatim: a timed-out + // round is still reported as the timeout it was. + const config = failingConfig(); + const { plugin, destroyed, state } = observable( 'hanging-plugin', () => new Promise(() => {}) ); @@ -340,22 +402,16 @@ describe('PluginHealthMonitor', () => { monitor.registerPlugin('hanging-plugin', config); monitor.startMonitoring('hanging-plugin', plugin); - // Round 1's guard rejects at +timeout. Below threshold: no restart. await vi.advanceTimersByTimeAsync(config.timeout); expect(monitor.getHealthStatus('hanging-plugin')).toBe('failed'); - expect(destroyed.count).toBe(0); - // Round 2 begins at +interval and its own guard rejects at +timeout. await vi.advanceTimersByTimeAsync(config.interval - config.timeout); await vi.advanceTimersByTimeAsync(config.timeout); - expect(destroyed.count).toBe(0); + await vi.advanceTimersByTimeAsync(FORMER_RESTART_BACKOFF_MS); - await vi.advanceTimersByTimeAsync(FIRST_RESTART_BACKOFF_MS); - expect(destroyed.count).toBe(1); - expect(monitor.getHealthStatus('hanging-plugin')).toBe('recovering'); + expect(destroyed.count).toBe(0); + expect(state.alive).toBe(true); - // The round is still reported as the timeout it was — restarting it does - // not relabel why it failed. const report = monitor.getHealthReport('hanging-plugin'); expect(report?.message).toBe(`Health check timeout after ${config.timeout}ms`); expect(report?.checks).toEqual([ @@ -365,58 +421,35 @@ describe('PluginHealthMonitor', () => { monitor.stopMonitoring('hanging-plugin'); }); - it('still restarts a plugin whose check RETURNS a failure', async () => { - // The route that already worked. Without this pin, unifying the two - // routes could close the throw gap by opening one here instead. - const config = restartConfig(); - const { plugin, destroyed } = restartable('unhealthy-plugin', async () => false); + it('leaves a plugin whose check RETURNS a failure alone', async () => { + // Was: 'still restarts a plugin whose check RETURNS a failure'. + const config = failingConfig(); + const { plugin, destroyed, state } = observable('unhealthy-plugin', async () => false); monitor.registerPlugin('unhealthy-plugin', config); monitor.startMonitoring('unhealthy-plugin', plugin); await vi.advanceTimersByTimeAsync(0); expect(monitor.getHealthStatus('unhealthy-plugin')).toBe('degraded'); - expect(destroyed.count).toBe(0); - - await vi.advanceTimersByTimeAsync(config.interval); - await vi.advanceTimersByTimeAsync(FIRST_RESTART_BACKOFF_MS); - expect(destroyed.count).toBe(1); - expect(monitor.getHealthStatus('unhealthy-plugin')).toBe('recovering'); - - monitor.stopMonitoring('unhealthy-plugin'); - }); - - it('leaves a throwing plugin alone when `autoRestart` is false', async () => { - // The control. Without it, the pins above would also pass if every - // failure restarted unconditionally — which would be a different defect, - // not a fix. - const config = restartConfig({ autoRestart: false }); - const { plugin, destroyed } = restartable('opted-out-plugin', async () => { - throw new Error('check exploded'); - }); - - monitor.registerPlugin('opted-out-plugin', config); - monitor.startMonitoring('opted-out-plugin', plugin); - await vi.advanceTimersByTimeAsync(0); await vi.advanceTimersByTimeAsync(config.interval); - await vi.advanceTimersByTimeAsync(FIRST_RESTART_BACKOFF_MS); + await vi.advanceTimersByTimeAsync(FORMER_RESTART_BACKOFF_MS); + expect(monitor.getHealthStatus('unhealthy-plugin')).toBe('unhealthy'); expect(destroyed.count).toBe(0); - expect(monitor.getHealthStatus('opted-out-plugin')).toBe('failed'); + expect(state.alive).toBe(true); - monitor.stopMonitoring('opted-out-plugin'); + monitor.stopMonitoring('unhealthy-plugin'); }); it('keeps a throw at `failed` immediately, with no threshold', async () => { - // The documented rule this fix deliberately does NOT unify away: - // "A check that throws — including one that exceeds `timeout` — is the - // separate `failed` status, applied immediately with no threshold" + // Unchanged from #11852 except for the retired config keys. The + // documented rule this must not unify away: "A check that throws — + // including one that exceeds `timeout` — is the separate `failed` + // status, applied immediately with no threshold" // (content/docs/protocol/kernel/lifecycle.mdx, "Custom Health Checks"). - // Sharing the counters and the restart decision must not turn a throw - // into the returned route's `degraded`. - const config = restartConfig({ failureThreshold: 10 }); - const { plugin, destroyed } = restartable('strict-plugin', async () => { + const config = failingConfig({ failureThreshold: 10 }); + const { plugin, destroyed } = observable('strict-plugin', async () => { throw new Error('check exploded'); }); @@ -431,6 +464,66 @@ describe('PluginHealthMonitor', () => { }); }); + // ── #12032 — the door for the audience that does not parse ─────────────── + // + // `PluginHealthCheckSchema` is not `.strict()`, so before the tombstones a + // leftover restart key was SILENTLY STRIPPED on the parse paths that exist. + // The tombstones answer the parse; `registerPlugin` answers everyone else — + // and everyone else is who there is, since nothing in the tree parses this + // schema outside its own unit test and `registerPlugin` takes the parsed + // shape straight from the caller's hand. + describe('a config still declaring a restart is REFUSED (#12032)', () => { + const legalConfig = (): PluginHealthCheckParsed => ({ + interval: 30_000, + timeout: 5_000, + failureThreshold: 3, + successThreshold: 1, + }); + + for (const [key, value] of [ + ['autoRestart', true], + ['maxRestartAttempts', 3], + ['restartBackoff', 'exponential'], + ] as const) { + it(`refuses \`${key}\` with an ADR-0112 envelope and the prescription`, () => { + const config = { ...legalConfig(), [key]: value } as PluginHealthCheckParsed; + + // ADR-0112: assert the ENVELOPE, not merely that it threw. A bare + // `toThrow()` here would stay green against an unrelated TypeError. + let caught: (Error & { code?: string; status?: number }) | undefined; + try { + monitor.registerPlugin('legacy-host-plugin', config); + } catch (error) { + caught = error as Error & { code?: string; status?: number }; + } + + expect(caught, `${key} must be refused`).toBeDefined(); + expect(caught?.code).toBe('VALIDATION_ERROR'); + expect(caught?.status).toBe(400); + + const message = caught?.message ?? ''; + expect(message).toContain(`'${key}' was removed`); + expect(message).toContain('#12032'); + expect(message).toContain('ADR-0049'); + expect(message).toContain('Delete the key'); + // The affordance that exists, named in place of the one that did not. + expect(message).toContain('getHealthStatus'); + + // Refused BEFORE anything was stored: a refused config must not leave + // a half-registered plugin behind. + expect(monitor.getHealthStatus('legacy-host-plugin')).toBeUndefined(); + expect(monitor.getAllHealthStatuses().size).toBe(0); + }); + } + + it('accepts a config that carries none of them (anti-vacuity)', () => { + // The control. Without it, the three refusals above would also pass if + // `registerPlugin` had simply started throwing for everything. + monitor.registerPlugin('clean-plugin', legalConfig()); + expect(monitor.getHealthStatus('clean-plugin')).toBe('unknown'); + }); + }); + // `successThreshold` is declared as "Consecutive successes needed to mark // healthy", and was read only while status was `unhealthy` or `degraded`. // The first success wrote `recovering` — a status the gate did not name — so @@ -449,8 +542,6 @@ describe('PluginHealthMonitor', () => { describe('`successThreshold` binds from every status that records a failure (#11955)', () => { const THRESHOLD = 3; const INTERVAL_MS = 10_000; - /** `calculateBackoff(0, 'fixed')` — the delay before the first restart. */ - const FIRST_RESTART_BACKOFF_MS = 1_000; const thresholdConfig = ( overrides: Partial = {} @@ -459,9 +550,6 @@ describe('PluginHealthMonitor', () => { timeout: 100, failureThreshold: 2, successThreshold: THRESHOLD, - autoRestart: false, - maxRestartAttempts: 3, - restartBackoff: 'fixed', checkMethod: 'healthCheck', ...overrides, }); @@ -575,33 +663,52 @@ describe('PluginHealthMonitor', () => { monitor.stopMonitoring('thrown-plugin'); }); - it('requires all three successes to leave the `recovering` a restart wrote', async () => { - // `recovering` reached from the restart path rather than from the - // success branch: `attemptRestart` writes it with both counters zeroed, - // so this is the status as a genuine STARTING point, not as a value the - // gate under test just produced. - const config = thresholdConfig({ failureThreshold: 1, autoRestart: true }); - const { plugin, mode, destroyed } = switchable('restarted-plugin', 'throw'); - - monitor.registerPlugin('restarted-plugin', config); - monitor.startMonitoring('restarted-plugin', plugin); + it('requires all three successes to leave a `recovering` it did not just write', async () => { + // [#12032] REWRITTEN, declared, not a quiet edit. This test was + // 'requires all three successes to leave the `recovering` a restart + // wrote', and its whole point was to reach `recovering` from the RESTART + // path — `attemptRestart` wrote it with both counters zeroed — so the + // gate under test was reading a status it had not just produced itself. + // That starting point no longer exists: the restart was only a + // `plugin.destroy()`, and it is gone (#12032, ADR-0049). The test drove + // a plugin to `destroyed=1, alive=false` and then asserted it walks to + // `healthy`, which is precisely the false report #12032 removes. + // + // What it was PROTECTING survives and is re-pinned here without the + // destroy: `recovering` as an INHERITED starting state rather than one + // the current round wrote. A fresh monitor is seeded by driving the + // plugin into `recovering` and then STOPPING — the next `startMonitoring` + // resumes from a `recovering` the gate did not just produce, with the + // success counter mid-flight, which is the same reading the restart path + // used to supply. + const config = thresholdConfig({ failureThreshold: 1 }); + const { plugin, mode, destroyed } = switchable('resumed-plugin', 'throw'); + + monitor.registerPlugin('resumed-plugin', config); + monitor.startMonitoring('resumed-plugin', plugin); await vi.advanceTimersByTimeAsync(0); - // The restart waits out `restartBackoff` before it destroys. - expect(destroyed.count).toBe(0); - await vi.advanceTimersByTimeAsync(FIRST_RESTART_BACKOFF_MS); - expect(destroyed.count).toBe(1); - expect(monitor.getHealthStatus('restarted-plugin')).toBe('recovering'); + expect(monitor.getHealthStatus('resumed-plugin')).toBe('failed'); mode.current = 'pass'; await nextRound(); - expect(monitor.getHealthStatus('restarted-plugin')).toBe('recovering'); - await nextRound(); - expect(monitor.getHealthStatus('restarted-plugin')).toBe('recovering'); + expect(monitor.getHealthStatus('resumed-plugin')).toBe('recovering'); + + // Suspend and resume: the status the next round reads was written by an + // earlier round, not by this one. + monitor.stopMonitoring('resumed-plugin'); + monitor.startMonitoring('resumed-plugin', plugin); + await vi.advanceTimersByTimeAsync(0); + expect(monitor.getHealthStatus('resumed-plugin')).toBe('recovering'); + await nextRound(); - expect(monitor.getHealthStatus('restarted-plugin')).toBe('healthy'); + expect(monitor.getHealthStatus('resumed-plugin')).toBe('healthy'); + + // And the reading the old version of this test could not make: the + // plugin that reached `healthy` was never torn down. + expect(destroyed.count).toBe(0); - monitor.stopMonitoring('restarted-plugin'); + monitor.stopMonitoring('resumed-plugin'); }); it('starts the count over after a throw interrupts a recovery (#11852)', async () => { diff --git a/packages/core/src/health-monitor.ts b/packages/core/src/health-monitor.ts index 9bb173ed0e..42f63f9da3 100644 --- a/packages/core/src/health-monitor.ts +++ b/packages/core/src/health-monitor.ts @@ -42,11 +42,132 @@ const RECOVERY_IS_THRESHOLD_GATED: Record = { unknown: false, }; +/** + * An ADR-0112-enveloped refusal (`code` + `status` on the error), so a caller — + * and a rejection-class test — can assert the refusal rather than merely "it + * threw". `VALIDATION_ERROR` is the standard catalog's generic + * argument-validation code, the same envelope `hot-reload.ts` uses for the + * sibling retirements (#12340, #12428). + */ +function healthMonitorRefusal(message: string): Error & { code: string; status: number } { + const err = new Error(message) as Error & { code: string; status: number }; + err.code = 'VALIDATION_ERROR'; + err.status = 400; + return err; +} + +/** + * Keys removed from `PluginHealthCheck` in 18 (#12032) that a host may still + * be passing. + * + * `PluginHealthCheckSchema` is not `.strict()`, so before the tombstones zod + * would have silently STRIPPED each of these — a clean parse and a setting + * that never takes effect. The tombstones answer the parse; this table is the + * door for the audience that does NOT parse, which is every host there is: + * nothing in the tree parses `PluginHealthCheckSchema` outside its own unit + * test, and `registerPlugin` takes the PARSED shape straight from the caller's + * hand. + * + * Each entry is the guidance clause; the `[HealthMonitor] Plugin '': ` + * prefix is added at throw time. The facts these must carry are pinned in + * `health-monitor.test.ts` by CONTENT, never by byte-equality against the + * spec-side prescriptions — `@objectstack/core`'s every import of + * `@objectstack/spec/kernel` is type-only, and a value import would be the + * first, linking that module's zod closure into every consumer of this package + * for three strings (the reasoning `hot-reload.ts` records for the same + * duplication). + */ +const RETIRED_HEALTH_CHECK_KEYS: ReadonlyArray = [ + [ + 'autoRestart', + "'autoRestart' was removed from PluginHealthCheck in @objectstack/spec 18 " + + '(#12032, ADR-0049 enforce-or-remove) — it never restarted a plugin. ' + + '`attemptRestart` called `plugin.destroy()` and stopped there, then ' + + "logged 'Plugin restarted' and set status `recovering`, and the periodic " + + 'checks carried on against the destroyed instance — which the default ' + + "check (`{ name: 'plugin-loaded', status: 'passed' }`) passes forever, so " + + 'a destroyed, never-re-initialised plugin ended up reported `healthy`. ' + + 'Delete the key. This monitor no longer destroys anything: a failing ' + + 'plugin is reported `unhealthy` or `failed` and left alone.', + ], + [ + 'maxRestartAttempts', + "'maxRestartAttempts' was removed from PluginHealthCheck in " + + '@objectstack/spec 18 (#12032, ADR-0049 enforce-or-remove) — it capped a ' + + 'restart that never happened, so it only counted `destroy()` calls. ' + + 'Delete the key.', + ], + [ + 'restartBackoff', + "'restartBackoff' was removed from PluginHealthCheck in @objectstack/spec " + + '18 (#12032, ADR-0049 enforce-or-remove) — it delayed a restart that ' + + 'never happened, so it only moved when the `destroy()` landed. Delete ' + + 'the key.', + ], +]; + +/** + * What a host does instead. Names only affordances that exist: the monitor + * could not restart a plugin even in principle, because `Plugin.init(ctx)` + * needs a `PluginContext` that only the kernel constructs and exposes to + * nobody (`ObjectKernel.context` is private, `KernelBase.createContext` is + * protected). + */ +const RESTART_IS_THE_HOSTS_JOB = + ' Restarting a plugin is the HOST\'s job in this host-driven library: poll ' + + '`getHealthStatus(pluginName)` / `getHealthReport(pluginName)` and act on ' + + '`unhealthy` / `failed` at the level that owns the plugin\'s lifetime — ' + + 'recreate the kernel, or let your supervisor restart the process.'; + +/** + * Refuse a key this library removed, at the moment the host hands the config + * over. First match wins; the order is the order they appear in the schema. + */ +function assertNoRetiredKeys(pluginName: string, config: object): void { + for (const [key, guidance] of RETIRED_HEALTH_CHECK_KEYS) { + if (!Object.prototype.hasOwnProperty.call(config, key)) { + continue; + } + throw healthMonitorRefusal( + `[HealthMonitor] Plugin '${pluginName}': ${guidance}${RESTART_IS_THE_HOSTS_JOB}` + ); + } +} + /** * Plugin Health Monitor - * - * Monitors plugin health status and performs automatic recovery actions. - * Implements the advanced lifecycle health monitoring protocol. + * + * Monitors plugin health status. It REPORTS; it does not act on what it finds. + * + * ## The monitor no longer "restarts" anything (#12032) + * + * It used to claim it did. `attemptRestart` called `plugin.destroy()` and + * stopped there — the comment above the call read "Call destroy and init to + * restart", and `init` appeared in this file ONLY inside that comment. What a + * plugin got was: destroy, a log line reading 'Plugin restarted', status + * `recovering`, and periodic checks continuing against the destroyed instance. + * The default check when no `checkMethod` resolves is + * `{ name: 'plugin-loaded', status: 'passed' }`, which a destroyed plugin + * passes indefinitely, so the TERMINAL report on a destroyed, never + * re-initialised plugin was `healthy` — and #11955 made that MORE convincing + * rather than less, because reaching `healthy` now costs `successThreshold` + * consecutive passing rounds. + * + * The restart could not be repaired in place. `Plugin.init(ctx)` needs a + * `PluginContext`, and the only two `plugin.init(...)` call sites in the tree + * are the kernel's own boot loops, over the full plugin list, with a context + * that is `private` on `ObjectKernel` and `protected` on `KernelBase`. No host + * can obtain one, so there was nothing for a re-init hook to call. ADR-0049 + * enforce-or-remove, with no roadmap to point EXPERIMENTAL at, therefore + * removed the declaration: `autoRestart`, `maxRestartAttempts` and + * `restartBackoff` are tombstoned in `@objectstack/spec` 18, and this class + * refuses a config that still carries one instead of accepting it and doing + * something else. + * + * What a failing plugin gets now is the truth: `degraded`, `unhealthy` or + * `failed`, and no destroy. Acting on that is the HOST's job — this is a + * host-driven library (#11825 route 2), and the host is the only party that + * owns the plugin's lifetime. */ export class PluginHealthMonitor { private logger: ObjectLogger; @@ -56,7 +177,6 @@ export class PluginHealthMonitor { private checkIntervals = new Map(); private failureCounters = new Map(); private successCounters = new Map(); - private restartAttempts = new Map(); constructor(logger: ObjectLogger) { this.logger = logger.child({ component: 'HealthMonitor' }); @@ -66,11 +186,16 @@ export class PluginHealthMonitor { * Register a plugin for health monitoring */ registerPlugin(pluginName: string, config: PluginHealthCheckParsed): void { + // Before anything is stored: a config still carrying a retired restart key + // is refused with its prescription, never accepted-and-ignored. Deliberately + // FIRST, so a config that would otherwise register cleanly cannot smuggle + // the false declaration past the door. + assertNoRetiredKeys(pluginName, config as object); + this.healthChecks.set(pluginName, config); this.healthStatus.set(pluginName, 'unknown'); this.failureCounters.set(pluginName, 0); this.successCounters.set(pluginName, 0); - this.restartAttempts.set(pluginName, 0); this.logger.info('Plugin registered for health monitoring', { plugin: pluginName, @@ -138,8 +263,8 @@ export class PluginHealthMonitor { let message: string | undefined; const checks: Array<{ name: string; status: 'passed' | 'failed' | 'warning'; message?: string }> = []; // Which failure route this round took, if any. A round can fail two - // disjoint ways and both settle here, so that the counters, the threshold - // and `autoRestart` are consulted in exactly one place below. + // disjoint ways and both settle here, so that the counters and the + // threshold are consulted in exactly one place below. let failureRoute: 'returned' | 'thrown' | undefined; try { @@ -204,12 +329,14 @@ export class PluginHealthMonitor { failureRoute = 'thrown'; } - // Both failure routes land here, and only here. Deliberately outside the - // `try`: `recordFailedRound` may await a restart, and a fault raised by the - // restart is not a health-check exception — catching it above would relabel - // it as one and push a second `health-check` entry for a check that ran. + // Both failure routes land here, and only here. Kept outside the `try`: + // a fault raised while RECORDING a round is not a health-check exception, + // and catching it above would relabel it as one and push a second + // `health-check` entry for a check that ran. (#11852 needed this because + // `recordFailedRound` awaited a restart; the restart is gone as of #12032 + // but the reason the boundary sits here is unchanged.) if (failureRoute) { - await this.recordFailedRound(pluginName, plugin, config, failureRoute); + this.recordFailedRound(pluginName, config, failureRoute); } // Create health report @@ -233,24 +360,26 @@ export class PluginHealthMonitor { * failure (`false` or `{ status: 'unhealthy' }`), or it *throws* — which by * `raceCheckTimeout` includes every `timeout` overrun, the severest case of * the two. The routes used to be handled in separate blocks, and only the - * returned one cleared `successCounters` or consulted `autoRestart`, so a - * plugin that hung until its timeout was marked `failed` and never restarted - * however `autoRestart` was set: the declared key covered only the milder - * half of the failures it names. + * returned one cleared `successCounters`, so the counters a declared + * `failureThreshold` / `successThreshold` are counted with depended on which + * way the round happened to fail (#11852). * * What stays route-specific is the *status label*, deliberately. A throw is * the separate `failed` status applied immediately with no threshold — that * is the documented contract (`content/docs/protocol/kernel/lifecycle.mdx`, * "Custom Health Checks") and is pinned by the timeout test. Only the - * counters and the restart decision are shared, because those are what - * `failureThreshold` and `autoRestart` declare, and neither names a route. + * counters are shared, because that is what `failureThreshold` declares, and + * it does not name a route. + * + * This round ENDS here. Nothing is done TO the plugin — see the #12032 note + * on the class: a monitor that cannot re-initialise a plugin has no business + * destroying one. */ - private async recordFailedRound( + private recordFailedRound( pluginName: string, - plugin: Plugin, config: PluginHealthCheckParsed, route: 'returned' | 'thrown' - ): Promise { + ): void { const failureCount = (this.failureCounters.get(pluginName) || 0) + 1; this.failureCounters.set(pluginName, failureCount); this.successCounters.set(pluginName, 0); @@ -268,84 +397,6 @@ export class PluginHealthMonitor { } else { this.healthStatus.set(pluginName, 'degraded'); } - - // Attempt auto-restart if configured — route-blind, by the same threshold. - if (thresholdReached && config.autoRestart) { - await this.attemptRestart(pluginName, plugin, config); - } - } - - /** - * Attempt to restart a plugin - */ - private async attemptRestart( - pluginName: string, - plugin: Plugin, - config: PluginHealthCheckParsed - ): Promise { - const attempts = this.restartAttempts.get(pluginName) || 0; - - if (attempts >= config.maxRestartAttempts) { - this.logger.error('Max restart attempts reached, giving up', { - plugin: pluginName, - attempts - }); - this.healthStatus.set(pluginName, 'failed'); - return; - } - - this.restartAttempts.set(pluginName, attempts + 1); - - // Calculate backoff delay - const delay = this.calculateBackoff(attempts, config.restartBackoff); - - this.logger.info('Scheduling plugin restart', { - plugin: pluginName, - attempt: attempts + 1, - delay - }); - - await new Promise(resolve => setTimeout(resolve, delay)); - - try { - // Call destroy and init to restart - if (plugin.destroy) { - await plugin.destroy(); - } - - // Note: Full restart would require kernel context - // This is a simplified version - actual implementation would need kernel integration - this.logger.info('Plugin restarted', { plugin: pluginName }); - - // Reset counters on successful restart - this.failureCounters.set(pluginName, 0); - this.successCounters.set(pluginName, 0); - this.healthStatus.set(pluginName, 'recovering'); - } catch (error) { - this.logger.error('Plugin restart failed', { - plugin: pluginName, - error - }); - this.healthStatus.set(pluginName, 'failed'); - } - } - - /** - * Calculate backoff delay for restarts - */ - private calculateBackoff(attempt: number, strategy: 'fixed' | 'linear' | 'exponential'): number { - const baseDelay = 1000; // 1 second base - - switch (strategy) { - case 'fixed': - return baseDelay; - case 'linear': - return baseDelay * (attempt + 1); - case 'exponential': - return baseDelay * Math.pow(2, attempt); - default: - return baseDelay; - } } /** @@ -383,7 +434,6 @@ export class PluginHealthMonitor { this.healthReports.clear(); this.failureCounters.clear(); this.successCounters.clear(); - this.restartAttempts.clear(); this.logger.info('Health monitor shutdown complete'); } diff --git a/packages/spec/authorable-defaults/kernel.json b/packages/spec/authorable-defaults/kernel.json index 738765cd55..8328219da3 100644 --- a/packages/spec/authorable-defaults/kernel.json +++ b/packages/spec/authorable-defaults/kernel.json @@ -95,11 +95,8 @@ "kernel/PluginCapability:certified = false", "kernel/PluginCapability:conformance = \"full\"", "kernel/PluginDependency:optional = false", - "kernel/PluginHealthCheck:autoRestart = false", "kernel/PluginHealthCheck:failureThreshold = 3", "kernel/PluginHealthCheck:interval = 30000", - "kernel/PluginHealthCheck:maxRestartAttempts = 3", - "kernel/PluginHealthCheck:restartBackoff = \"exponential\"", "kernel/PluginHealthCheck:successThreshold = 1", "kernel/PluginHealthCheck:timeout = 5000", "kernel/PluginInstallConfig:autoUpdate = false", diff --git a/packages/spec/authorable-surface/kernel.json b/packages/spec/authorable-surface/kernel.json index 664b19c751..7ce51a0b06 100644 --- a/packages/spec/authorable-surface/kernel.json +++ b/packages/spec/authorable-surface/kernel.json @@ -502,12 +502,12 @@ "kernel/PluginDependencyResolutionResult:warnings", "kernel/PluginEngines:platform", "kernel/PluginEngines:protocol", - "kernel/PluginHealthCheck:autoRestart", + "kernel/PluginHealthCheck:autoRestart [RETIRED]", "kernel/PluginHealthCheck:checkMethod", "kernel/PluginHealthCheck:failureThreshold", "kernel/PluginHealthCheck:interval", - "kernel/PluginHealthCheck:maxRestartAttempts", - "kernel/PluginHealthCheck:restartBackoff", + "kernel/PluginHealthCheck:maxRestartAttempts [RETIRED]", + "kernel/PluginHealthCheck:restartBackoff [RETIRED]", "kernel/PluginHealthCheck:successThreshold", "kernel/PluginHealthCheck:timeout", "kernel/PluginHealthReport:checks", diff --git a/packages/spec/src/kernel/plugin-lifecycle-advanced.test.ts b/packages/spec/src/kernel/plugin-lifecycle-advanced.test.ts index 2ba42479d6..2bc223877d 100644 --- a/packages/spec/src/kernel/plugin-lifecycle-advanced.test.ts +++ b/packages/spec/src/kernel/plugin-lifecycle-advanced.test.ts @@ -31,26 +31,84 @@ describe('Plugin Lifecycle Advanced Schemas', () => { expect(healthCheck.timeout).toBe(5000); expect(healthCheck.failureThreshold).toBe(3); expect(healthCheck.successThreshold).toBe(1); - expect(healthCheck.autoRestart).toBe(false); - expect(healthCheck.maxRestartAttempts).toBe(3); - expect(healthCheck.restartBackoff).toBe('exponential'); + // [#12032] The three restart defaults ASSERTED HERE ARE GONE — declared, + // not a quiet edit. `autoRestart` (false), `maxRestartAttempts` (3) and + // `restartBackoff` ('exponential') were pinned on this line and the + // assertions passed precisely BECAUSE the keys parsed and the thing they + // named never happened. They are tombstoned; their refusal is pinned + // below. + expect(healthCheck).not.toHaveProperty('autoRestart'); + expect(healthCheck).not.toHaveProperty('maxRestartAttempts'); + expect(healthCheck).not.toHaveProperty('restartBackoff'); }); it('should validate custom health check configuration', () => { + // [#12032] `autoRestart: true`, `maxRestartAttempts: 5` and + // `restartBackoff: 'linear'` REMOVED from this fixture — declared, not a + // quiet edit. Their presence here was the strongest single pin of the + // false contract: the fixture asserted, via `toEqual`, that a config + // asking for automatic restart survives the parse intact — which was + // true right up to the moment it stopped meaning anything at runtime, + // and it never meant anything at runtime. const config = { interval: 60000, timeout: 10000, failureThreshold: 5, successThreshold: 2, - autoRestart: true, - maxRestartAttempts: 5, - restartBackoff: 'linear' as const, checkMethod: 'healthCheck', }; const healthCheck = PluginHealthCheckSchema.parse(config); expect(healthCheck).toEqual(config); }); + // ── [#12032] The three restart keys are REFUSED, with the prescription ─── + // + // Tombstones, not deletions: `PluginHealthCheckSchema` is not `.strict()`, + // so a bare deletion would be a SILENT STRIP (#3733, ADR-0104) — a clean + // parse and a setting that never takes effect, which is a milder form of + // the defect being retired. + const RETIRED_RESTART_KEYS = { + autoRestart: true, + maxRestartAttempts: 5, + restartBackoff: 'linear', + } as const; + + for (const [key, value] of Object.entries(RETIRED_RESTART_KEYS)) { + it(`refuses ${key} with the retirement prescription (#12032)`, () => { + const result = PluginHealthCheckSchema.safeParse( + { [key]: value } as Record + ); + expect(result.success, `${key} must no longer parse`).toBe(false); + + // The message IS the contract — it is the whole migration document for + // whoever hits it. Assert the load-bearing clauses, not the bytes. + const message = result.success ? '' : result.error.issues[0]?.message ?? ''; + expect(message).toContain('was removed'); + expect(message).toContain('#12032'); + expect(message).toContain('ADR-0049'); + expect(message).toContain('Delete the key'); + // The measured fact, in every one of the three prescriptions: the + // restart was only ever a destroy. + expect(message).toContain('plugin.destroy()'); + expect(message).toContain('healthy'); + // And the affordance that DOES exist, named in place of the one that + // never did. + expect(message).toContain('getHealthStatus'); + }); + } + + it('leaves an unrelated unknown key alone (anti-vacuity)', () => { + // The control. `PluginHealthCheckSchema` is not `.strict()`, so an + // unknown key is stripped rather than refused — which is exactly why the + // three above needed tombstones. Without this, the refusals could be + // passing because the schema had become strict, a different change. + const result = PluginHealthCheckSchema.safeParse( + { somethingElse: true } as Record + ); + expect(result.success).toBe(true); + expect(result.success && result.data).not.toHaveProperty('somethingElse'); + }); + it('should enforce minimum interval', () => { expect(() => PluginHealthCheckSchema.parse({ interval: 500 })).toThrow(); }); diff --git a/packages/spec/src/kernel/plugin-lifecycle-advanced.zod.ts b/packages/spec/src/kernel/plugin-lifecycle-advanced.zod.ts index 1d54c2f20b..184ea7c495 100644 --- a/packages/spec/src/kernel/plugin-lifecycle-advanced.zod.ts +++ b/packages/spec/src/kernel/plugin-lifecycle-advanced.zod.ts @@ -32,6 +32,70 @@ export const PluginHealthStatusSchema = lazySchema(() => z.enum([ 'unknown', // Health status cannot be determined ]).describe('Current health status of the plugin')); +/** + * Prescriptions for the three restart keys retired in 18 (#12032). + * + * Deliberately carry NO `os migrate meta --from 17` sentence, for exactly the + * reason `HOT_RELOAD_STATE_STRATEGY_RETIRED` and + * `HOT_RELOAD_WATCH_PATTERNS_RETIRED` below do not: that command replays the + * conversion chain over authored METADATA SOURCES, and `PluginHealthCheck` is + * not an authorable surface — it is a library parameter a host passes to + * `PluginHealthMonitor` in TypeScript (the #4914 / #11825 keep). No authored + * document has ever been able to carry these keys, so naming the command would + * promise an affordance that cannot apply, which is the same false-promise + * defect ADR-0049 exists to prevent. The migrate-sentence pin judges only + * prescriptions that DO name the command, so this absence is in scope by + * construction rather than by exemption. + * + * The three retire together because two of them are the VOCABULARY OF the + * first: with no restart, "maximum restart attempts" and "backoff strategy for + * restart delays" have nothing left to be the vocabulary of — the same reason + * `distributedConfig` could not honestly outlive the `stateStrategy` value it + * was documented as being required for (#12340). + */ +const RESTART_NOT_IMPLEMENTED = + 'A `PluginHealthMonitor` never restarted anything. `attemptRestart` called ' + + "`plugin.destroy()` and stopped there — the in-source comment said \"Call " + + 'destroy and init to restart", but `init` appeared in `health-monitor.ts` ' + + 'ONLY inside that comment. What a plugin actually got was: destroy, a log ' + + "line reading 'Plugin restarted', status `recovering`, and periodic health " + + 'checks continuing against the destroyed instance — which the default ' + + "check (`{ name: 'plugin-loaded', status: 'passed' }`, used whenever no " + + '`checkMethod` resolves) passes forever, so the terminal report on a ' + + 'destroyed, never-re-initialised plugin was `healthy`.'; + +/** What a host does instead. Names only affordances that exist. */ +const RESTART_REPLACEMENT = + 'Restarting a plugin is the HOST\'s job in this host-driven library, and ' + + 'the monitor could not do it even in principle: `Plugin.init(ctx)` needs a ' + + '`PluginContext`, which only the kernel constructs and which it exposes to ' + + 'nobody (`ObjectKernel.context` is private; `KernelBase.createContext` is ' + + 'protected). Poll `getHealthStatus(pluginName)` / ' + + '`getHealthReport(pluginName)` and act on `unhealthy` / `failed` at the ' + + 'level that owns the plugin\'s lifetime — recreate the kernel, or let your ' + + 'supervisor restart the process. The monitor reports; it does not act.'; + +const AUTO_RESTART_RETIRED = + '`PluginHealthCheck.autoRestart` was removed in @objectstack/spec 18 ' + + '(#12032, ADR-0049 enforce-or-remove) — it never restarted a plugin. ' + + RESTART_NOT_IMPLEMENTED + + ' Delete the key. ' + RESTART_REPLACEMENT; + +const MAX_RESTART_ATTEMPTS_RETIRED = + '`PluginHealthCheck.maxRestartAttempts` was removed in @objectstack/spec 18 ' + + '(#12032, ADR-0049 enforce-or-remove) — it capped a restart that never ' + + 'happened. ' + RESTART_NOT_IMPLEMENTED + + ' The cap counted destroy calls, so raising it only scheduled further ' + + '"restarts" of a plugin that was never brought back up. Delete the key. ' + + RESTART_REPLACEMENT; + +const RESTART_BACKOFF_RETIRED = + '`PluginHealthCheck.restartBackoff` was removed in @objectstack/spec 18 ' + + '(#12032, ADR-0049 enforce-or-remove) — it delayed a restart that never ' + + 'happened. ' + RESTART_NOT_IMPLEMENTED + + ' The chosen strategy only moved when the destroy landed. Delete the key. ' + + RESTART_REPLACEMENT; + /** * Plugin Health Check Configuration * Defines how to check plugin health @@ -68,22 +132,26 @@ export const PluginHealthCheckSchema = lazySchema(() => z.object({ .describe('Method name to call for health check'), /** - * Enable automatic restart on failure + * REMOVED in 18 (#12032) — tombstoned, not deleted. + * + * This object is not `.strict()`, so a bare deletion would be a SILENT + * STRIP (#3733, ADR-0104): a clean parse and a setting that never takes + * effect — which is a milder version of the very defect being retired. + * The tombstone makes the removal audible on both channels: `tsc` types + * the key `never`, and a value that reaches the parse raises the + * prescription itself rather than a generic unrecognised-key error. */ - autoRestart: z.boolean().default(false) - .describe('Automatically restart plugin on health check failure'), - + autoRestart: retiredKey(AUTO_RESTART_RETIRED), + /** - * Maximum number of restart attempts + * REMOVED in 18 (#12032) — the cap on a restart that never happened. */ - maxRestartAttempts: z.number().int().min(0).default(3) - .describe('Maximum restart attempts before giving up'), - + maxRestartAttempts: retiredKey(MAX_RESTART_ATTEMPTS_RETIRED), + /** - * Backoff strategy for restarts + * REMOVED in 18 (#12032) — the delay before a restart that never happened. */ - restartBackoff: z.enum(['fixed', 'linear', 'exponential']).default('exponential') - .describe('Backoff strategy for restart delays'), + restartBackoff: retiredKey(RESTART_BACKOFF_RETIRED), })); /** @@ -400,6 +468,68 @@ export const HotReloadConfigSchema = lazySchema(() => z.object({ // declaration. Degradation / update-strategy vocabularies return only via the // ENFORCE route of ADR-0049 through a new ADR: the implementation first, then // a declaration of exactly what it honours. +// +// ── [#12032] AMENDED 2026-08-26: the restart that was only a destroy ──────── +// +// The keep is STILL intact — `PluginHealthCheckSchema` and +// `PluginHealthMonitor` stay. `autoRestart`, `maxRestartAttempts` and +// `restartBackoff` are REMOVED, on the same per-key test #12340 and #12428 +// applied one file over, and for the sharper reason: this key HAD a reader +// that acted, and what it did was not what the key declared. +// +// What was measured, at ee3595cefd: +// +// `attemptRestart` called `plugin.destroy()` and stopped. The comment above +// the call read "Call destroy and init to restart"; `init` appeared in +// `health-monitor.ts` ONLY inside that comment. The sequence a plugin got +// was destroy -> `logger.info('Plugin restarted')` -> status `recovering` +// -> periodic checks continuing against the destroyed instance. The default +// check when no `checkMethod` resolves is +// `{ name: 'plugin-loaded', status: 'passed' }`, which a destroyed plugin +// passes indefinitely, so the TERMINAL report was `healthy`. Reproduced +// before anything was changed, with `successThreshold: 3`: +// +// round 1 (failing): status=failed destroyed=0 alive=true +// after backoff: status=recovering destroyed=1 alive=false +// recovery round 1: status=recovering destroyed=1 alive=false +// recovery round 2: status=recovering destroyed=1 alive=false +// recovery round 3: status=healthy destroyed=1 alive=false +// +// #11955 made that worse rather than better: reaching `healthy` now costs +// `successThreshold` CONSECUTIVE passing rounds, so the false report is more +// convincing, not less. +// +// Why REMOVE and not the other two ADR-0049 states. ENFORCE would have to +// BUILD the restart, and the class cannot host one: `Plugin.init(ctx)` needs a +// `PluginContext`, and the only two `plugin.init(...)` call sites in the tree +// are the kernel's own boot loops (`kernel-base.ts:202`, `kernel.ts:607`), +// both over the full plugin list with a context that is `private` on +// `ObjectKernel` and `protected` on `KernelBase` — no host can obtain one +// (positive control: the same scan resolves five real non-test +// `plugin.destroy()` call sites, so it sees lifecycle drivers). Building a +// per-plugin re-init API plus a host callback for a caller that does not exist +// is exactly the speculation ADR-0049's staged decision names as the wrong +// default at this milestone. EXPERIMENTAL requires a roadmap, and a scan of +// the whole `docs/` planning + ADR corpus returned ZERO mentions of plugin +// auto-restart against 118 control hits for "health" and 13 for "hot reload" +// in the same corpus. +// +// The other two keys leave with it, not as a tidy-up: with no restart, +// "Maximum restart attempts before giving up" and "Backoff strategy for +// restart delays" have nothing left to be the vocabulary OF — the same test +// that took `distributedConfig` out with the `stateStrategy` value it was +// documented as being required for (#12340). +// +// Route: TOMBSTONE, for #12428's reason — a key leaving a SURVIVING def has +// no route-3 exit, and this object is not `.strict()`, so a bare deletion is +// a silent strip. All three are registered by exact key in +// RETIRED_KEYS_BY_MAJOR[18]; their surface lines carry `[RETIRED]` rather +// than disappearing, and the def still emits, so `api-surface` and +// `json-schema.manifest` do not move. The tombstone answers the parse; the +// registration-time refusal in `PluginHealthMonitor.registerPlugin` answers +// the audience that does NOT parse — a host handing the object straight to +// the class, which is every host there is. The D3 semantic entry +// `plugin-auto-restart-never-reinitialised` records the reasoning. // ──────────────────────────────────────────────────────────────────────────── /** diff --git a/packages/spec/src/migrations/entries/retired-keys/18.kernel__PluginHealthCheck__autoRestart.ts b/packages/spec/src/migrations/entries/retired-keys/18.kernel__PluginHealthCheck__autoRestart.ts new file mode 100644 index 0000000000..c3c587e6c1 --- /dev/null +++ b/packages/spec/src/migrations/entries/retired-keys/18.kernel__PluginHealthCheck__autoRestart.ts @@ -0,0 +1,55 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +// #12032 — ADR-0049 enforce-or-remove, one class over from #12428 (PR #12571) +// and #12340 (PR #12425) in the same host-driven lifecycle library, and for a +// sharper reason than either: this key HAD a reader that acted, and what it did +// was not what the key declared. +// +// `PluginHealthMonitor.attemptRestart` called `plugin.destroy()` and stopped. +// The comment above the call read "Call destroy and init to restart"; `init` +// appeared in `health-monitor.ts` ONLY inside that comment. What a plugin got +// was destroy -> `logger.info('Plugin restarted')` -> status `recovering` -> +// periodic checks continuing against the destroyed instance, which the default +// check (`{ name: 'plugin-loaded', status: 'passed' }`, used whenever no +// `checkMethod` resolves) passes forever. So the TERMINAL report on a +// destroyed, never-re-initialised plugin was `healthy` — reproduced before +// anything was changed, at ee3595cefd with `successThreshold: 3`: +// +// round 1 (failing): status=failed destroyed=0 alive=true +// after backoff: status=recovering destroyed=1 alive=false +// recovery round 3: status=healthy destroyed=1 alive=false +// +// #11955 made that MORE convincing rather than less: reaching `healthy` now +// costs `successThreshold` consecutive passing rounds. +// +// Neither of ADR-0049's other two states was available. ENFORCE would have to +// BUILD the restart, and the class cannot host one: `Plugin.init(ctx)` needs a +// `PluginContext`, and the only two `plugin.init(...)` call sites in the tree +// are the kernel's own boot loops (`kernel-base.ts:202`, `kernel.ts:607`), both +// over the full plugin list with a context that is `private` on `ObjectKernel` +// and `protected` on `KernelBase` — no host can obtain one (positive control: +// the same scan resolves five real non-test `plugin.destroy()` call sites, so +// it sees lifecycle drivers). Building a per-plugin re-init API plus a host +// callback for a caller that does not exist is the speculation ADR-0049's +// staged decision names as the wrong default at this milestone. EXPERIMENTAL +// requires a roadmap, and a scan of the whole `docs/` planning + ADR corpus +// returned ZERO mentions of plugin auto-restart against 118 control hits for +// "health" and 13 for "hot reload" in the same corpus. +// +// The three keys retire together: with no restart, "Maximum restart attempts +// before giving up" and "Backoff strategy for restart delays" have nothing left +// to be the vocabulary OF — the same test that took `distributedConfig` out +// with the `stateStrategy` value it was documented as being required for. +// +// Tombstoned with `retiredKey()` rather than deleted, for #12428's reason: a +// key leaving a SURVIVING def has no route-3 exit, and `PluginHealthCheckSchema` +// is not `.strict()`, so a bare deletion would be a SILENT STRIP (#3733, +// ADR-0104). Deliberately NO D2 conversion: the chain walks a normalized STACK, +// and `PluginHealthCheck` is not an authorable surface — no metadata-type +// binding, stack collection or manifest embed ever carried it, and nothing in +// the tree parses `PluginHealthCheckSchema` outside its own unit test — so a +// conversion would be a transform with no seam that ever runs. The D3 semantic +// entry `plugin-auto-restart-never-reinitialised` is the declaration, and the +// registration-time refusal in `PluginHealthMonitor.registerPlugin` is the door +// for the audience that exists. +export const entry = 'kernel/PluginHealthCheck:autoRestart'; diff --git a/packages/spec/src/migrations/entries/retired-keys/18.kernel__PluginHealthCheck__maxRestartAttempts.ts b/packages/spec/src/migrations/entries/retired-keys/18.kernel__PluginHealthCheck__maxRestartAttempts.ts new file mode 100644 index 0000000000..d0b9686eb1 --- /dev/null +++ b/packages/spec/src/migrations/entries/retired-keys/18.kernel__PluginHealthCheck__maxRestartAttempts.ts @@ -0,0 +1,55 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +// #12032 — ADR-0049 enforce-or-remove, one class over from #12428 (PR #12571) +// and #12340 (PR #12425) in the same host-driven lifecycle library, and for a +// sharper reason than either: this key HAD a reader that acted, and what it did +// was not what the key declared. +// +// `PluginHealthMonitor.attemptRestart` called `plugin.destroy()` and stopped. +// The comment above the call read "Call destroy and init to restart"; `init` +// appeared in `health-monitor.ts` ONLY inside that comment. What a plugin got +// was destroy -> `logger.info('Plugin restarted')` -> status `recovering` -> +// periodic checks continuing against the destroyed instance, which the default +// check (`{ name: 'plugin-loaded', status: 'passed' }`, used whenever no +// `checkMethod` resolves) passes forever. So the TERMINAL report on a +// destroyed, never-re-initialised plugin was `healthy` — reproduced before +// anything was changed, at ee3595cefd with `successThreshold: 3`: +// +// round 1 (failing): status=failed destroyed=0 alive=true +// after backoff: status=recovering destroyed=1 alive=false +// recovery round 3: status=healthy destroyed=1 alive=false +// +// #11955 made that MORE convincing rather than less: reaching `healthy` now +// costs `successThreshold` consecutive passing rounds. +// +// Neither of ADR-0049's other two states was available. ENFORCE would have to +// BUILD the restart, and the class cannot host one: `Plugin.init(ctx)` needs a +// `PluginContext`, and the only two `plugin.init(...)` call sites in the tree +// are the kernel's own boot loops (`kernel-base.ts:202`, `kernel.ts:607`), both +// over the full plugin list with a context that is `private` on `ObjectKernel` +// and `protected` on `KernelBase` — no host can obtain one (positive control: +// the same scan resolves five real non-test `plugin.destroy()` call sites, so +// it sees lifecycle drivers). Building a per-plugin re-init API plus a host +// callback for a caller that does not exist is the speculation ADR-0049's +// staged decision names as the wrong default at this milestone. EXPERIMENTAL +// requires a roadmap, and a scan of the whole `docs/` planning + ADR corpus +// returned ZERO mentions of plugin auto-restart against 118 control hits for +// "health" and 13 for "hot reload" in the same corpus. +// +// The three keys retire together: with no restart, "Maximum restart attempts +// before giving up" and "Backoff strategy for restart delays" have nothing left +// to be the vocabulary OF — the same test that took `distributedConfig` out +// with the `stateStrategy` value it was documented as being required for. +// +// Tombstoned with `retiredKey()` rather than deleted, for #12428's reason: a +// key leaving a SURVIVING def has no route-3 exit, and `PluginHealthCheckSchema` +// is not `.strict()`, so a bare deletion would be a SILENT STRIP (#3733, +// ADR-0104). Deliberately NO D2 conversion: the chain walks a normalized STACK, +// and `PluginHealthCheck` is not an authorable surface — no metadata-type +// binding, stack collection or manifest embed ever carried it, and nothing in +// the tree parses `PluginHealthCheckSchema` outside its own unit test — so a +// conversion would be a transform with no seam that ever runs. The D3 semantic +// entry `plugin-auto-restart-never-reinitialised` is the declaration, and the +// registration-time refusal in `PluginHealthMonitor.registerPlugin` is the door +// for the audience that exists. +export const entry = 'kernel/PluginHealthCheck:maxRestartAttempts'; diff --git a/packages/spec/src/migrations/entries/retired-keys/18.kernel__PluginHealthCheck__restartBackoff.ts b/packages/spec/src/migrations/entries/retired-keys/18.kernel__PluginHealthCheck__restartBackoff.ts new file mode 100644 index 0000000000..6080bde43d --- /dev/null +++ b/packages/spec/src/migrations/entries/retired-keys/18.kernel__PluginHealthCheck__restartBackoff.ts @@ -0,0 +1,55 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +// #12032 — ADR-0049 enforce-or-remove, one class over from #12428 (PR #12571) +// and #12340 (PR #12425) in the same host-driven lifecycle library, and for a +// sharper reason than either: this key HAD a reader that acted, and what it did +// was not what the key declared. +// +// `PluginHealthMonitor.attemptRestart` called `plugin.destroy()` and stopped. +// The comment above the call read "Call destroy and init to restart"; `init` +// appeared in `health-monitor.ts` ONLY inside that comment. What a plugin got +// was destroy -> `logger.info('Plugin restarted')` -> status `recovering` -> +// periodic checks continuing against the destroyed instance, which the default +// check (`{ name: 'plugin-loaded', status: 'passed' }`, used whenever no +// `checkMethod` resolves) passes forever. So the TERMINAL report on a +// destroyed, never-re-initialised plugin was `healthy` — reproduced before +// anything was changed, at ee3595cefd with `successThreshold: 3`: +// +// round 1 (failing): status=failed destroyed=0 alive=true +// after backoff: status=recovering destroyed=1 alive=false +// recovery round 3: status=healthy destroyed=1 alive=false +// +// #11955 made that MORE convincing rather than less: reaching `healthy` now +// costs `successThreshold` consecutive passing rounds. +// +// Neither of ADR-0049's other two states was available. ENFORCE would have to +// BUILD the restart, and the class cannot host one: `Plugin.init(ctx)` needs a +// `PluginContext`, and the only two `plugin.init(...)` call sites in the tree +// are the kernel's own boot loops (`kernel-base.ts:202`, `kernel.ts:607`), both +// over the full plugin list with a context that is `private` on `ObjectKernel` +// and `protected` on `KernelBase` — no host can obtain one (positive control: +// the same scan resolves five real non-test `plugin.destroy()` call sites, so +// it sees lifecycle drivers). Building a per-plugin re-init API plus a host +// callback for a caller that does not exist is the speculation ADR-0049's +// staged decision names as the wrong default at this milestone. EXPERIMENTAL +// requires a roadmap, and a scan of the whole `docs/` planning + ADR corpus +// returned ZERO mentions of plugin auto-restart against 118 control hits for +// "health" and 13 for "hot reload" in the same corpus. +// +// The three keys retire together: with no restart, "Maximum restart attempts +// before giving up" and "Backoff strategy for restart delays" have nothing left +// to be the vocabulary OF — the same test that took `distributedConfig` out +// with the `stateStrategy` value it was documented as being required for. +// +// Tombstoned with `retiredKey()` rather than deleted, for #12428's reason: a +// key leaving a SURVIVING def has no route-3 exit, and `PluginHealthCheckSchema` +// is not `.strict()`, so a bare deletion would be a SILENT STRIP (#3733, +// ADR-0104). Deliberately NO D2 conversion: the chain walks a normalized STACK, +// and `PluginHealthCheck` is not an authorable surface — no metadata-type +// binding, stack collection or manifest embed ever carried it, and nothing in +// the tree parses `PluginHealthCheckSchema` outside its own unit test — so a +// conversion would be a transform with no seam that ever runs. The D3 semantic +// entry `plugin-auto-restart-never-reinitialised` is the declaration, and the +// registration-time refusal in `PluginHealthMonitor.registerPlugin` is the door +// for the audience that exists. +export const entry = 'kernel/PluginHealthCheck:restartBackoff'; diff --git a/packages/spec/src/migrations/entries/semantic/18.plugin-auto-restart-never-reinitialised.ts b/packages/spec/src/migrations/entries/semantic/18.plugin-auto-restart-never-reinitialised.ts new file mode 100644 index 0000000000..5c3da787bc --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.plugin-auto-restart-never-reinitialised.ts @@ -0,0 +1,92 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +export const entry: SemanticMigration = { + id: 'plugin-auto-restart-never-reinitialised', + surface: + '`PluginHealthCheck.autoRestart`, `PluginHealthCheck.maxRestartAttempts` ' + + 'and `PluginHealthCheck.restartBackoff`, and the ' + + '`PluginHealthMonitor.attemptRestart` path that read them', + replacement: + 'Poll `PluginHealthMonitor.getHealthStatus(pluginName)` / ' + + '`getHealthReport(pluginName)` and act on `unhealthy` / `failed` in the ' + + 'HOST. There is no in-tree replacement for the keys, because restarting ' + + 'a plugin is the host\'s job in this host-driven library and the monitor ' + + 'could not do it even in principle: `Plugin.init(ctx)` needs a ' + + '`PluginContext`, which only the kernel constructs and which it exposes ' + + 'to nobody (`ObjectKernel.context` is private, `KernelBase.createContext` ' + + 'is protected). Recreate the kernel, or let your supervisor restart the ' + + 'process — whichever level actually owns the plugin\'s lifetime. The ' + + 'monitor reports; it does not act.', + reason: + 'ADR-0049 enforce-or-remove, applied one class over from #12428 and ' + + '#12340 in the same host-driven lifecycle library, and for a sharper ' + + 'reason than either: this key HAD a reader that acted, and what it did ' + + 'was not what the key declared. `attemptRestart` called ' + + '`plugin.destroy()` and stopped there. The comment above the call read ' + + '"Call destroy and init to restart", and `init` appeared in ' + + '`health-monitor.ts` ONLY inside that comment. So what a plugin actually ' + + "got was: destroy, a log line reading 'Plugin restarted', status " + + '`recovering`, and periodic health checks continuing to run against the ' + + 'destroyed instance — which the default check when no `checkMethod` ' + + "resolves (`{ name: 'plugin-loaded', status: 'passed' }`) passes " + + 'indefinitely. The TERMINAL report on a destroyed, never-re-initialised ' + + 'plugin was therefore `healthy`, reproduced at ee3595cefd with ' + + '`successThreshold: 3` as failed -> recovering (destroyed=1, alive=false) ' + + '-> recovering -> recovering -> healthy (destroyed=1, alive=false). ' + + '#11955 made that MORE convincing rather than less, because reaching ' + + '`healthy` now costs `successThreshold` CONSECUTIVE passing rounds, so ' + + 'the plugin has to earn a declared number of passes to be misreported. ' + + 'Meanwhile `restartAttempts` was incremented as though a restart had ' + + 'occurred, and `maxRestartAttempts` / `restartBackoff` scheduled further ' + + '"restarts" of a plugin that was never brought back up. Neither of the ' + + 'other two ADR-0049 states was available: ENFORCE would have to BUILD ' + + 'the restart, and the class cannot host one — `Plugin.init(ctx)` needs a ' + + '`PluginContext`, and the only two `plugin.init(...)` call sites in the ' + + 'tree are the kernel\'s own boot loops over the full plugin list, with a ' + + 'context that is private on `ObjectKernel` and protected on ' + + '`KernelBase`, so a host-provided re-init hook would have had nothing to ' + + 'call (positive control: the same scan resolves five real non-test ' + + '`plugin.destroy()` call sites, so it sees lifecycle drivers). Building ' + + 'that API for a caller that does not exist — no runtime constructs ' + + '`PluginHealthMonitor` (#11825) — is exactly the speculation ADR-0049\'s ' + + 'staged decision names as the wrong default at this milestone, where the ' + + 'shippable liability is the false promise and not the missing feature. ' + + 'EXPERIMENTAL requires a roadmap, and a scan of the whole `docs/` ' + + 'planning + ADR corpus returned ZERO mentions of plugin auto-restart ' + + 'against 118 control hits for "health" and 13 for "hot reload" in the ' + + 'same corpus. The other two keys leave with the first rather than as a ' + + 'tidy-up: with no restart, "Maximum restart attempts before giving up" ' + + 'and "Backoff strategy for restart delays" have nothing left to be the ' + + 'vocabulary OF — the same test that took `distributedConfig` out with ' + + 'the `stateStrategy` value it was documented as being required for ' + + '(#12340). All three are TOMBSTONED rather than deleted, for #12428\'s ' + + 'reason: a key leaving a SURVIVING def has no route-3 exit, and ' + + '`PluginHealthCheckSchema` is not `.strict()`, so a bare deletion would ' + + 'be a silent strip (#3733, ADR-0104) — a milder form of the very defect ' + + 'being retired. There is no D2 conversion, because `PluginHealthCheck` ' + + 'is not an authorable surface: no metadata-type binding, stack ' + + 'collection or manifest embed ever carried it, so there is no authored ' + + 'document to rewrite. This entry IS the declaration.', + acceptanceCriteria: + 'No host passes `autoRestart`, `maxRestartAttempts` or `restartBackoff` to ' + + '`PluginHealthMonitor.registerPlugin`. TypeScript hosts cannot: all ' + + 'three are typed `never` by the tombstones. JavaScript hosts, and config ' + + 'that arrived as JSON, get a loud refusal carrying the prescription — an ' + + 'ADR-0112 envelope (`code: VALIDATION_ERROR`, `status: 400`), thrown ' + + 'BEFORE any state is stored so a refused config cannot leave a ' + + 'half-registered plugin behind. `PluginHealthMonitor` no longer calls ' + + '`plugin.destroy()` at all: `attemptRestart` and `calculateBackoff` are ' + + 'gone with the `restartAttempts` counter, and a plugin that crosses ' + + '`failureThreshold` is reported `degraded` / `unhealthy` / `failed` and ' + + 'left running. The rest of the monitor is UNCHANGED: registration, ' + + 'periodic checks, the `timeout` race and its refd-timer guard (#4875), ' + + 'the two failure routes sharing counters (#11852), and ' + + '`successThreshold` binding from every status that records a failure ' + + '(#11955) all behave exactly as before — `recovering` is now written ' + + 'only by the success branch, which is the one writer that ever meant it. ' + + 'The #11825 keep still stands: `PluginHealthCheckSchema` still exports ' + + 'from `./kernel` and `PluginHealthMonitor` still exports from ' + + '`@objectstack/core` with its tests green.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index c34052d9bf..56f7102274 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -6477,6 +6477,94 @@ const step18: MigrationStep = { + 'drift window carrying `indexes[].where` is rejected with the database-layer prescription ' + 'rather than saved with the key silently dropped.', }, + { + id: 'plugin-auto-restart-never-reinitialised', + surface: + '`PluginHealthCheck.autoRestart`, `PluginHealthCheck.maxRestartAttempts` ' + + 'and `PluginHealthCheck.restartBackoff`, and the ' + + '`PluginHealthMonitor.attemptRestart` path that read them', + replacement: + 'Poll `PluginHealthMonitor.getHealthStatus(pluginName)` / ' + + '`getHealthReport(pluginName)` and act on `unhealthy` / `failed` in the ' + + 'HOST. There is no in-tree replacement for the keys, because restarting ' + + 'a plugin is the host\'s job in this host-driven library and the monitor ' + + 'could not do it even in principle: `Plugin.init(ctx)` needs a ' + + '`PluginContext`, which only the kernel constructs and which it exposes ' + + 'to nobody (`ObjectKernel.context` is private, `KernelBase.createContext` ' + + 'is protected). Recreate the kernel, or let your supervisor restart the ' + + 'process — whichever level actually owns the plugin\'s lifetime. The ' + + 'monitor reports; it does not act.', + reason: + 'ADR-0049 enforce-or-remove, applied one class over from #12428 and ' + + '#12340 in the same host-driven lifecycle library, and for a sharper ' + + 'reason than either: this key HAD a reader that acted, and what it did ' + + 'was not what the key declared. `attemptRestart` called ' + + '`plugin.destroy()` and stopped there. The comment above the call read ' + + '"Call destroy and init to restart", and `init` appeared in ' + + '`health-monitor.ts` ONLY inside that comment. So what a plugin actually ' + + "got was: destroy, a log line reading 'Plugin restarted', status " + + '`recovering`, and periodic health checks continuing to run against the ' + + 'destroyed instance — which the default check when no `checkMethod` ' + + "resolves (`{ name: 'plugin-loaded', status: 'passed' }`) passes " + + 'indefinitely. The TERMINAL report on a destroyed, never-re-initialised ' + + 'plugin was therefore `healthy`, reproduced at ee3595cefd with ' + + '`successThreshold: 3` as failed -> recovering (destroyed=1, alive=false) ' + + '-> recovering -> recovering -> healthy (destroyed=1, alive=false). ' + + '#11955 made that MORE convincing rather than less, because reaching ' + + '`healthy` now costs `successThreshold` CONSECUTIVE passing rounds, so ' + + 'the plugin has to earn a declared number of passes to be misreported. ' + + 'Meanwhile `restartAttempts` was incremented as though a restart had ' + + 'occurred, and `maxRestartAttempts` / `restartBackoff` scheduled further ' + + '"restarts" of a plugin that was never brought back up. Neither of the ' + + 'other two ADR-0049 states was available: ENFORCE would have to BUILD ' + + 'the restart, and the class cannot host one — `Plugin.init(ctx)` needs a ' + + '`PluginContext`, and the only two `plugin.init(...)` call sites in the ' + + 'tree are the kernel\'s own boot loops over the full plugin list, with a ' + + 'context that is private on `ObjectKernel` and protected on ' + + '`KernelBase`, so a host-provided re-init hook would have had nothing to ' + + 'call (positive control: the same scan resolves five real non-test ' + + '`plugin.destroy()` call sites, so it sees lifecycle drivers). Building ' + + 'that API for a caller that does not exist — no runtime constructs ' + + '`PluginHealthMonitor` (#11825) — is exactly the speculation ADR-0049\'s ' + + 'staged decision names as the wrong default at this milestone, where the ' + + 'shippable liability is the false promise and not the missing feature. ' + + 'EXPERIMENTAL requires a roadmap, and a scan of the whole `docs/` ' + + 'planning + ADR corpus returned ZERO mentions of plugin auto-restart ' + + 'against 118 control hits for "health" and 13 for "hot reload" in the ' + + 'same corpus. The other two keys leave with the first rather than as a ' + + 'tidy-up: with no restart, "Maximum restart attempts before giving up" ' + + 'and "Backoff strategy for restart delays" have nothing left to be the ' + + 'vocabulary OF — the same test that took `distributedConfig` out with ' + + 'the `stateStrategy` value it was documented as being required for ' + + '(#12340). All three are TOMBSTONED rather than deleted, for #12428\'s ' + + 'reason: a key leaving a SURVIVING def has no route-3 exit, and ' + + '`PluginHealthCheckSchema` is not `.strict()`, so a bare deletion would ' + + 'be a silent strip (#3733, ADR-0104) — a milder form of the very defect ' + + 'being retired. There is no D2 conversion, because `PluginHealthCheck` ' + + 'is not an authorable surface: no metadata-type binding, stack ' + + 'collection or manifest embed ever carried it, so there is no authored ' + + 'document to rewrite. This entry IS the declaration.', + acceptanceCriteria: + 'No host passes `autoRestart`, `maxRestartAttempts` or `restartBackoff` to ' + + '`PluginHealthMonitor.registerPlugin`. TypeScript hosts cannot: all ' + + 'three are typed `never` by the tombstones. JavaScript hosts, and config ' + + 'that arrived as JSON, get a loud refusal carrying the prescription — an ' + + 'ADR-0112 envelope (`code: VALIDATION_ERROR`, `status: 400`), thrown ' + + 'BEFORE any state is stored so a refused config cannot leave a ' + + 'half-registered plugin behind. `PluginHealthMonitor` no longer calls ' + + '`plugin.destroy()` at all: `attemptRestart` and `calculateBackoff` are ' + + 'gone with the `restartAttempts` counter, and a plugin that crosses ' + + '`failureThreshold` is reported `degraded` / `unhealthy` / `failed` and ' + + 'left running. The rest of the monitor is UNCHANGED: registration, ' + + 'periodic checks, the `timeout` race and its refd-timer guard (#4875), ' + + 'the two failure routes sharing counters (#11852), and ' + + '`successThreshold` binding from every status that records a failure ' + + '(#11955) all behave exactly as before — `recovering` is now written ' + + 'only by the success branch, which is the one writer that ever meant it. ' + + 'The #11825 keep still stands: `PluginHealthCheckSchema` still exports ' + + 'from `./kernel` and `PluginHealthMonitor` still exports from ' + + '`@objectstack/core` with its tests green.', + }, { id: 'plugin-manifest-contributes-dead-members-retired', surface: @@ -7625,6 +7713,165 @@ export const RETIRED_KEYS_BY_MAJOR: Readonly> // The prescription reaches authors through the tombstone (`tsc` + the parse) // and the D3 semantic entry `metadata-plugin-additional-types-retired`. 'kernel/MetadataPluginConfig:additionalTypes', + // #12032 — ADR-0049 enforce-or-remove, one class over from #12428 (PR #12571) + // and #12340 (PR #12425) in the same host-driven lifecycle library, and for a + // sharper reason than either: this key HAD a reader that acted, and what it did + // was not what the key declared. + // + // `PluginHealthMonitor.attemptRestart` called `plugin.destroy()` and stopped. + // The comment above the call read "Call destroy and init to restart"; `init` + // appeared in `health-monitor.ts` ONLY inside that comment. What a plugin got + // was destroy -> `logger.info('Plugin restarted')` -> status `recovering` -> + // periodic checks continuing against the destroyed instance, which the default + // check (`{ name: 'plugin-loaded', status: 'passed' }`, used whenever no + // `checkMethod` resolves) passes forever. So the TERMINAL report on a + // destroyed, never-re-initialised plugin was `healthy` — reproduced before + // anything was changed, at ee3595cefd with `successThreshold: 3`: + // + // round 1 (failing): status=failed destroyed=0 alive=true + // after backoff: status=recovering destroyed=1 alive=false + // recovery round 3: status=healthy destroyed=1 alive=false + // + // #11955 made that MORE convincing rather than less: reaching `healthy` now + // costs `successThreshold` consecutive passing rounds. + // + // Neither of ADR-0049's other two states was available. ENFORCE would have to + // BUILD the restart, and the class cannot host one: `Plugin.init(ctx)` needs a + // `PluginContext`, and the only two `plugin.init(...)` call sites in the tree + // are the kernel's own boot loops (`kernel-base.ts:202`, `kernel.ts:607`), both + // over the full plugin list with a context that is `private` on `ObjectKernel` + // and `protected` on `KernelBase` — no host can obtain one (positive control: + // the same scan resolves five real non-test `plugin.destroy()` call sites, so + // it sees lifecycle drivers). Building a per-plugin re-init API plus a host + // callback for a caller that does not exist is the speculation ADR-0049's + // staged decision names as the wrong default at this milestone. EXPERIMENTAL + // requires a roadmap, and a scan of the whole `docs/` planning + ADR corpus + // returned ZERO mentions of plugin auto-restart against 118 control hits for + // "health" and 13 for "hot reload" in the same corpus. + // + // The three keys retire together: with no restart, "Maximum restart attempts + // before giving up" and "Backoff strategy for restart delays" have nothing left + // to be the vocabulary OF — the same test that took `distributedConfig` out + // with the `stateStrategy` value it was documented as being required for. + // + // Tombstoned with `retiredKey()` rather than deleted, for #12428's reason: a + // key leaving a SURVIVING def has no route-3 exit, and `PluginHealthCheckSchema` + // is not `.strict()`, so a bare deletion would be a SILENT STRIP (#3733, + // ADR-0104). Deliberately NO D2 conversion: the chain walks a normalized STACK, + // and `PluginHealthCheck` is not an authorable surface — no metadata-type + // binding, stack collection or manifest embed ever carried it, and nothing in + // the tree parses `PluginHealthCheckSchema` outside its own unit test — so a + // conversion would be a transform with no seam that ever runs. The D3 semantic + // entry `plugin-auto-restart-never-reinitialised` is the declaration, and the + // registration-time refusal in `PluginHealthMonitor.registerPlugin` is the door + // for the audience that exists. + 'kernel/PluginHealthCheck:autoRestart', + // #12032 — ADR-0049 enforce-or-remove, one class over from #12428 (PR #12571) + // and #12340 (PR #12425) in the same host-driven lifecycle library, and for a + // sharper reason than either: this key HAD a reader that acted, and what it did + // was not what the key declared. + // + // `PluginHealthMonitor.attemptRestart` called `plugin.destroy()` and stopped. + // The comment above the call read "Call destroy and init to restart"; `init` + // appeared in `health-monitor.ts` ONLY inside that comment. What a plugin got + // was destroy -> `logger.info('Plugin restarted')` -> status `recovering` -> + // periodic checks continuing against the destroyed instance, which the default + // check (`{ name: 'plugin-loaded', status: 'passed' }`, used whenever no + // `checkMethod` resolves) passes forever. So the TERMINAL report on a + // destroyed, never-re-initialised plugin was `healthy` — reproduced before + // anything was changed, at ee3595cefd with `successThreshold: 3`: + // + // round 1 (failing): status=failed destroyed=0 alive=true + // after backoff: status=recovering destroyed=1 alive=false + // recovery round 3: status=healthy destroyed=1 alive=false + // + // #11955 made that MORE convincing rather than less: reaching `healthy` now + // costs `successThreshold` consecutive passing rounds. + // + // Neither of ADR-0049's other two states was available. ENFORCE would have to + // BUILD the restart, and the class cannot host one: `Plugin.init(ctx)` needs a + // `PluginContext`, and the only two `plugin.init(...)` call sites in the tree + // are the kernel's own boot loops (`kernel-base.ts:202`, `kernel.ts:607`), both + // over the full plugin list with a context that is `private` on `ObjectKernel` + // and `protected` on `KernelBase` — no host can obtain one (positive control: + // the same scan resolves five real non-test `plugin.destroy()` call sites, so + // it sees lifecycle drivers). Building a per-plugin re-init API plus a host + // callback for a caller that does not exist is the speculation ADR-0049's + // staged decision names as the wrong default at this milestone. EXPERIMENTAL + // requires a roadmap, and a scan of the whole `docs/` planning + ADR corpus + // returned ZERO mentions of plugin auto-restart against 118 control hits for + // "health" and 13 for "hot reload" in the same corpus. + // + // The three keys retire together: with no restart, "Maximum restart attempts + // before giving up" and "Backoff strategy for restart delays" have nothing left + // to be the vocabulary OF — the same test that took `distributedConfig` out + // with the `stateStrategy` value it was documented as being required for. + // + // Tombstoned with `retiredKey()` rather than deleted, for #12428's reason: a + // key leaving a SURVIVING def has no route-3 exit, and `PluginHealthCheckSchema` + // is not `.strict()`, so a bare deletion would be a SILENT STRIP (#3733, + // ADR-0104). Deliberately NO D2 conversion: the chain walks a normalized STACK, + // and `PluginHealthCheck` is not an authorable surface — no metadata-type + // binding, stack collection or manifest embed ever carried it, and nothing in + // the tree parses `PluginHealthCheckSchema` outside its own unit test — so a + // conversion would be a transform with no seam that ever runs. The D3 semantic + // entry `plugin-auto-restart-never-reinitialised` is the declaration, and the + // registration-time refusal in `PluginHealthMonitor.registerPlugin` is the door + // for the audience that exists. + 'kernel/PluginHealthCheck:maxRestartAttempts', + // #12032 — ADR-0049 enforce-or-remove, one class over from #12428 (PR #12571) + // and #12340 (PR #12425) in the same host-driven lifecycle library, and for a + // sharper reason than either: this key HAD a reader that acted, and what it did + // was not what the key declared. + // + // `PluginHealthMonitor.attemptRestart` called `plugin.destroy()` and stopped. + // The comment above the call read "Call destroy and init to restart"; `init` + // appeared in `health-monitor.ts` ONLY inside that comment. What a plugin got + // was destroy -> `logger.info('Plugin restarted')` -> status `recovering` -> + // periodic checks continuing against the destroyed instance, which the default + // check (`{ name: 'plugin-loaded', status: 'passed' }`, used whenever no + // `checkMethod` resolves) passes forever. So the TERMINAL report on a + // destroyed, never-re-initialised plugin was `healthy` — reproduced before + // anything was changed, at ee3595cefd with `successThreshold: 3`: + // + // round 1 (failing): status=failed destroyed=0 alive=true + // after backoff: status=recovering destroyed=1 alive=false + // recovery round 3: status=healthy destroyed=1 alive=false + // + // #11955 made that MORE convincing rather than less: reaching `healthy` now + // costs `successThreshold` consecutive passing rounds. + // + // Neither of ADR-0049's other two states was available. ENFORCE would have to + // BUILD the restart, and the class cannot host one: `Plugin.init(ctx)` needs a + // `PluginContext`, and the only two `plugin.init(...)` call sites in the tree + // are the kernel's own boot loops (`kernel-base.ts:202`, `kernel.ts:607`), both + // over the full plugin list with a context that is `private` on `ObjectKernel` + // and `protected` on `KernelBase` — no host can obtain one (positive control: + // the same scan resolves five real non-test `plugin.destroy()` call sites, so + // it sees lifecycle drivers). Building a per-plugin re-init API plus a host + // callback for a caller that does not exist is the speculation ADR-0049's + // staged decision names as the wrong default at this milestone. EXPERIMENTAL + // requires a roadmap, and a scan of the whole `docs/` planning + ADR corpus + // returned ZERO mentions of plugin auto-restart against 118 control hits for + // "health" and 13 for "hot reload" in the same corpus. + // + // The three keys retire together: with no restart, "Maximum restart attempts + // before giving up" and "Backoff strategy for restart delays" have nothing left + // to be the vocabulary OF — the same test that took `distributedConfig` out + // with the `stateStrategy` value it was documented as being required for. + // + // Tombstoned with `retiredKey()` rather than deleted, for #12428's reason: a + // key leaving a SURVIVING def has no route-3 exit, and `PluginHealthCheckSchema` + // is not `.strict()`, so a bare deletion would be a SILENT STRIP (#3733, + // ADR-0104). Deliberately NO D2 conversion: the chain walks a normalized STACK, + // and `PluginHealthCheck` is not an authorable surface — no metadata-type + // binding, stack collection or manifest embed ever carried it, and nothing in + // the tree parses `PluginHealthCheckSchema` outside its own unit test — so a + // conversion would be a transform with no seam that ever runs. The D3 semantic + // entry `plugin-auto-restart-never-reinitialised` is the declaration, and the + // registration-time refusal in `PluginHealthMonitor.registerPlugin` is the door + // for the audience that exists. + 'kernel/PluginHealthCheck:restartBackoff', // #9220 — ADR-0049 enforce-or-remove at ELEMENT grain. `element:filter` never // had a renderer or reader anywhere: objectui registers none (its // renderers/basic/elements.tsx header deferred the element to "owning plugins" From ea9a6c90c1785de2a0d4ae4a17bd3578fde3ac04 Mon Sep 17 00:00:00 2001 From: Warren Buffett Date: Wed, 26 Aug 2026 13:11:14 +0000 Subject: [PATCH 2/2] test(core): pin the terminal state on its own, so an ablation fails on the contract The richer walk-the-sequence pin trips one of its early status assertions first, so under ablation it reports 'expected recovering to be failed' rather than the contract. This one asserts nothing in between: it drives the failure round, the former backoff window and successThreshold consecutive passes, then reads what an operator reads. Under the re-introduced destroy it fails on exactly 'the monitor reported `healthy` for a plugin that had been destroyed'. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01W6HFzyH98W1YaQXhJUJt6o --- packages/core/src/health-monitor.test.ts | 40 ++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/packages/core/src/health-monitor.test.ts b/packages/core/src/health-monitor.test.ts index f65b6504a9..1bfde77979 100644 --- a/packages/core/src/health-monitor.test.ts +++ b/packages/core/src/health-monitor.test.ts @@ -360,6 +360,46 @@ describe('PluginHealthMonitor', () => { monitor.stopMonitoring('observed-plugin'); }); + // ⭐⭐ The same contract with NOTHING else in the way. The pin above walks + // the sequence and checks each step, which means an ablation trips one of + // its early assertions first; this one asserts only what an operator reads + // at the END, so the assertion that fails IS the contract: + // + // "a plugin reported `healthy` is a plugin that is alive." + // + // Deliberately NOT "`init` was called" — that pin goes green the moment + // somebody adds a no-op `init`, while this one cannot: it reads the + // plugin's own liveness, which only a real re-initialisation restores. + it('a plugin reported `healthy` is a plugin that is alive', async () => { + const config = failingConfig({ failureThreshold: 1, successThreshold: 3 }); + const mode = { current: 'throw' as 'throw' | 'pass' }; + const { plugin, state } = observable('terminal-state-plugin', async () => { + if (mode.current === 'throw') throw new Error('check exploded'); + return true; + }); + + monitor.registerPlugin('terminal-state-plugin', config); + monitor.startMonitoring('terminal-state-plugin', plugin); + + // The failing round, the window the destroy used to land in, then + // `successThreshold` consecutive passes. No assertions in between. + await vi.advanceTimersByTimeAsync(0); + await vi.advanceTimersByTimeAsync(FORMER_RESTART_BACKOFF_MS); + mode.current = 'pass'; + for (let round = 1; round <= config.successThreshold; round++) { + await vi.advanceTimersByTimeAsync(INTERVAL_MS); + await vi.advanceTimersByTimeAsync(0); + } + + expect(monitor.getHealthStatus('terminal-state-plugin')).toBe('healthy'); + expect( + state.alive, + 'the monitor reported `healthy` for a plugin that had been destroyed' + ).toBe(true); + + monitor.stopMonitoring('terminal-state-plugin'); + }); + it('leaves a plugin whose check THROWS alone, however many rounds fail', async () => { // Was: 'restarts a plugin whose check THROWS, once failureThreshold // accumulates', asserting destroyed.count === 1 and `recovering`.