Skip to content

Commit b72db01

Browse files
os-warrenclaude
andauthored
fix(spec,core): PluginHealthMonitor stops claiming a restart it never performed; the three PluginHealthCheck restart keys retired (#12032, ADR-0049) (#12589)
* fix(spec,core): PluginHealthMonitor stops claiming a restart it never performed; the three PluginHealthCheck restart keys retired (#12032, ADR-0049) `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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W6HFzyH98W1YaQXhJUJt6o * 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W6HFzyH98W1YaQXhJUJt6o --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent da1126a commit b72db01

15 files changed

Lines changed: 1257 additions & 260 deletions
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/core": minor
4+
---
5+
6+
fix(spec,core): `PluginHealthMonitor` stops claiming a restart it never performed; the three `PluginHealthCheck` restart keys retired (#12032, ADR-0049)
7+
8+
<!-- adr-0087: registered plugin-auto-restart-never-reinitialised -->
9+
10+
**BREAKING** accept-set narrowing, landing after the v17.0.0 cut (the lockstep
11+
launch-window convention ships it as `minor`; the prescriptions are registered
12+
under protocol major 18 — three `RETIRED_KEYS_BY_MAJOR[18]` entries plus the D3
13+
semantic entry `plugin-auto-restart-never-reinitialised` — where
14+
`os migrate meta` users will look). Graded `minor` rather than `major` for the
15+
same reason #12340 and #12428 were, the day before, in this same module.
16+
17+
## What was measured
18+
19+
`PluginHealthMonitor.attemptRestart` called `plugin.destroy()` and stopped
20+
there. The comment above the call read *"Call destroy and init to restart"*,
21+
and `init` appeared in `health-monitor.ts` **only inside that comment**. So a
22+
plugin whose health checks crossed `failureThreshold` with `autoRestart: true`
23+
got: `destroy()`, a log line reading `Plugin restarted`, status `recovering`,
24+
and periodic health checks that carried on running against the destroyed
25+
instance. The default check when no `checkMethod` resolves is
26+
`{ name: 'plugin-loaded', status: 'passed' }`, which a destroyed object passes
27+
indefinitely — so the **terminal** report on a torn-down, never-re-initialised
28+
plugin was `healthy`.
29+
30+
Reproduced at `ee3595cefd` before anything was changed, with
31+
`successThreshold: 3`:
32+
33+
```
34+
round 1 (failing): status=failed destroyed=0 alive=true
35+
after backoff: status=recovering destroyed=1 alive=false
36+
recovery round 1: status=recovering destroyed=1 alive=false
37+
recovery round 2: status=recovering destroyed=1 alive=false
38+
recovery round 3: status=healthy destroyed=1 alive=false
39+
```
40+
41+
#11955 made that report *more* convincing rather than less: reaching `healthy`
42+
now costs `successThreshold` consecutive passing rounds, so a destroyed plugin
43+
has to earn a declared number of passes before it is misreported.
44+
`restartAttempts` was incremented as though a restart had occurred, and
45+
`maxRestartAttempts` / `restartBackoff` scheduled further "restarts" of a plugin
46+
that was never brought back up.
47+
48+
## Why REMOVE and not the other two ADR-0049 states
49+
50+
**ENFORCE** would have to build the restart, and the class cannot host one.
51+
`Plugin.init(ctx)` needs a `PluginContext`; the only two `plugin.init(...)` call
52+
sites in the tree are the kernel's own boot loops (`kernel-base.ts:202`,
53+
`kernel.ts:607`), both over the full plugin list, with a context that is
54+
`private` on `ObjectKernel` and `protected` on `KernelBase`. No host can obtain
55+
one, so a host-provided re-init hook would have had nothing to call. (Positive
56+
control for that scan: the same pass resolves five real non-test
57+
`plugin.destroy()` call sites, so it does see lifecycle drivers.) Building a
58+
per-plugin re-init API for a caller that does not exist — no runtime constructs
59+
`PluginHealthMonitor` (#11825) — is the speculation ADR-0049's staged decision
60+
names as the wrong default at this milestone, where the shippable liability is
61+
the false promise and not the missing feature.
62+
63+
**EXPERIMENTAL** requires a roadmap. A scan of the whole `docs/` planning + ADR
64+
corpus returned **zero** mentions of plugin auto-restart, against 118 control
65+
hits for "health" and 13 for "hot reload" in the same corpus.
66+
67+
`maxRestartAttempts` and `restartBackoff` leave with `autoRestart` rather than
68+
as a tidy-up: with no restart, *"Maximum restart attempts before giving up"* and
69+
*"Backoff strategy for restart delays"* have nothing left to be the vocabulary
70+
**of** — the test that took `distributedConfig` out with the `stateStrategy`
71+
value it was documented as requiring (#12340).
72+
73+
## What changes for a host
74+
75+
All three keys are **tombstoned**, not deleted: `PluginHealthCheckSchema` is not
76+
`.strict()`, so a bare deletion would be a silent strip (#3733, ADR-0104) — a
77+
milder form of the defect being retired. A TypeScript host gets a `tsc` error
78+
(the keys are typed `never`); a parse raises the prescription; and
79+
`PluginHealthMonitor.registerPlugin` refuses a hand-built config carrying any of
80+
them with an ADR-0112 envelope (`code: VALIDATION_ERROR`, `status: 400`), thrown
81+
before any state is stored so a refused config leaves no half-registered plugin
82+
behind.
83+
84+
`PluginHealthMonitor` no longer calls `plugin.destroy()` at all. A plugin that
85+
crosses `failureThreshold` is reported `degraded` / `unhealthy` / `failed` and
86+
left running; acting on that is the host's job in this host-driven library
87+
(#11825 route 2). Poll `getHealthStatus(pluginName)` / `getHealthReport(pluginName)`
88+
and restart at the level that owns the plugin's lifetime.
89+
90+
Everything else in the monitor is unchanged: registration, periodic checks, the
91+
`timeout` race and its refd-timer guard (#4875), both failure routes sharing the
92+
counters (#11852), and `successThreshold` binding from every status that records
93+
a failure (#11955). `recovering` is now written only by the success branch —
94+
the one writer that ever meant it.

content/docs/protocol/kernel/lifecycle.mdx

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -693,8 +693,7 @@ export const salesforcePlugin = {
693693
const monitor = new PluginHealthMonitor(kernel.logger);
694694

695695
// `registerPlugin` takes the PARSED config, so parse it: the schema fills in
696-
// interval 30000, timeout 5000, failureThreshold 3, successThreshold 1,
697-
// autoRestart false, maxRestartAttempts 3, restartBackoff 'exponential'.
696+
// interval 30000, timeout 5000, failureThreshold 3 and successThreshold 1.
698697
monitor.registerPlugin(
699698
salesforcePlugin.name,
700699
PluginHealthCheckSchema.parse({ checkMethod: 'healthCheck' }),
@@ -734,9 +733,26 @@ failure resets the success count to zero — both routes included — so a throw
734733
part-way through a recovery starts the next attempt at one rather than resuming
735734
where it left off. The symmetry holds the other way too: a passing round resets
736735
the failure count, so `failureThreshold` likewise counts only an unbroken run.
737-
A successful auto-restart lands the plugin in `recovering` with **both**
738-
counters cleared, so a restarted plugin still owes a full `successThreshold` of
739-
passing rounds before it reads `healthy`.
736+
737+
### The monitor reports; it does not act
738+
739+
Nothing above does anything **to** the plugin. A failing plugin is labelled
740+
`degraded`, `unhealthy` or `failed` and left running; the monitor never calls
741+
`destroy()`, and acting on what it reports is the host's job — this is a
742+
host-driven library, and the host is the only party that owns the plugin's
743+
lifetime.
744+
745+
It used to claim otherwise. `PluginHealthCheck` carried `autoRestart`,
746+
`maxRestartAttempts` and `restartBackoff`, and a plugin that crossed
747+
`failureThreshold` with `autoRestart: true` got `plugin.destroy()` called on it
748+
— and nothing else. `init()` was never called, because the monitor has no
749+
`PluginContext` to call it with and no way to obtain one. The plugin was then
750+
logged as `Plugin restarted`, marked `recovering`, and kept under periodic
751+
checks it went on passing from the grave: the default check when no
752+
`checkMethod` resolves is `plugin-loaded`, which a destroyed object satisfies
753+
forever. So the terminal report on a torn-down plugin was `healthy`. The three
754+
keys were removed in `@objectstack/spec` 18 under ADR-0049 enforce-or-remove;
755+
`PluginHealthMonitor.registerPlugin` refuses a config that still carries one.
740756

741757
At the default `successThreshold: 1` none of this is observable: the first
742758
passing round satisfies the count from every status, and `recovering` is never

content/docs/references/kernel/plugin-lifecycle-advanced.mdx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,9 @@ const result = HotReloadConfigSchema.parse(data);
6363
| **failureThreshold** | `integer` | optional (default: `3`) | Consecutive failures needed to mark unhealthy |
6464
| **successThreshold** | `integer` | optional (default: `1`) | Consecutive successes needed to mark healthy |
6565
| **checkMethod** | `string` | optional | Method name to call for health check |
66-
| **autoRestart** | `boolean` | optional (default: `false`) | Automatically restart plugin on health check failure |
67-
| **maxRestartAttempts** | `integer` | optional (default: `3`) | Maximum restart attempts before giving up |
68-
| **restartBackoff** | `Enum<'fixed' \| 'linear' \| 'exponential'>` | optional (default: `"exponential"`) | Backoff strategy for restart delays |
66+
| **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. |
67+
| **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. |
68+
| **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. |
6969

7070

7171
---

packages/core/examples/phase2-integration.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -270,9 +270,10 @@ async function example() {
270270
timeout: 5000,
271271
failureThreshold: 3,
272272
successThreshold: 1,
273-
autoRestart: true,
274-
maxRestartAttempts: 3,
275-
restartBackoff: 'exponential',
273+
// [#12032] `autoRestart` / `maxRestartAttempts` / `restartBackoff`
274+
// removed: the monitor never restarted anything (it called
275+
// `plugin.destroy()` and reported the corpse `healthy`), so the keys
276+
// were retired under ADR-0049. Act on `getHealthStatus()` in the host.
276277
},
277278

278279
// Hot reload

0 commit comments

Comments
 (0)