fix(spec,core): PluginHealthMonitor stops claiming a restart it never performed; the three PluginHealthCheck restart keys retired (#12032, ADR-0049) - #12589
Conversation
… 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
…n 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
📓 Docs Drift CheckThis PR changes 2 package(s): 1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
What this run could not see
Coarse fallback — 132 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 590b84844f39752798d137cd8862ea85a90223a1 && git checkout 590b84844f39752798d137cd8862ea85a90223a1
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin da1126a032b175f39606fc1ddb86642d3cdad64f ea9a6c90c1785de2a0d4ae4a17bd3578fde3ac04 && git checkout -B drift-repro da1126a032b175f39606fc1ddb86642d3cdad64f && git merge --no-ff ea9a6c90c1785de2a0d4ae4a17bd3578fde3ac04
node scripts/docs-audit/affected-docs.mjs --json da1126a032b175f39606fc1ddb86642d3cdad64f
|
PM review — accepted, flipped ready, auto-merge armed.
|
Fixes #12032
PluginHealthMonitor.attemptRestartcalledplugin.destroy()and stopped there. The comment above the call read "Call destroy and init to restart", andinitappeared inhealth-monitor.tsonly inside that comment. So a plugin that crossedfailureThresholdwithautoRestart: truegot:destroy(), a log line readingPlugin restarted, statusrecovering, and periodic health checks that carried on running against the destroyed instance.The reproduction — the terminal state, not the missing call
The default check when no
checkMethodresolves is{ name: 'plugin-loaded', status: 'passed' }, which a destroyed object passes indefinitely. Measured atee3595cefdbefore anything was changed, withsuccessThreshold: 3:The terminal report on a torn-down, never-re-initialised plugin was
healthy. #11955 made that more convincing rather than less: reachinghealthynow costssuccessThresholdconsecutive passing rounds, so the plugin has to earn a declared number of passes before it is misreported.The route, decided by measurement
Route 2 — stop claiming a restart — taken to its coherent end: the declaration goes, and the monitor stops destroying.
ENFORCE was not available.
Plugin.init(ctx)needs aPluginContext. The only twoplugin.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 isprivateonObjectKernelandprotectedonKernelBase—createContext()isprotectedtoo, and there is no public accessor. A host-provided re-init hook would have had nothing to call. Positive control for that scan: the same pass resolves five real non-testplugin.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 constructsPluginHealthMonitor(#11825) — is the speculation ADR-0049's staged decision names as the wrong default at this milestone, where "the real, shippable liability is the false promise, 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.PluginHealthMonitoritself: 0 hits.So: REMOVE.
autoRestart,maxRestartAttemptsandrestartBackoffare tombstoned in@objectstack/spec18. The latter two 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, exactly the test that tookdistributedConfigout with thestateStrategyvalue it was documented as requiring (#12340).This follows #12340 (PR #12425) and #12428 (PR #12571) one class over, with one difference worth naming: those keys had no reader. This one had a reader that acted — and what it did was not what the key declared.
Citations re-taken on the post-#12336 ref
packages/spec/src/kernel/plugin-lifecycle-advanced.zod.ts:73(pre-change) —autoRestart: z.boolean().default(false).describe('Automatically restart plugin on health check failure'). Still present, text unchanged; the card cited it without a line number and it holds.content/docs/references/kernel/plugin-lifecycle-advanced.mdx:114— moved. TheautoRestartrow is at line 66 on currentorigin/main, and the page is now auto-generated from the zod describe strings (⚠️ AUTO-GENERATED — DO NOT EDIT). Same text, different line and different provenance.content/docs/protocol/kernel/lifecycle.mdx:737— a citation the card did not have, and the one that mattered most: the hand-written protocol page taught the false contract — "A successful auto-restart lands the plugin inrecoveringwith both counters cleared, so a restarted plugin still owes a fullsuccessThresholdof passing rounds before it readshealthy." Corrected here.Clause ② — accept/reject behaviour on a shipped surface: YES, on two doors
Not "only what is logged and what status is recorded". Evidence,
git diff --statagainst the merge baseee3595cefd:The parse. An accept-set narrowing: three keys that parsed now reject.
The ratchets moved with it: three
[RETIRED]markers inauthorable-surface/kernel.json, three defaults dropped fromauthorable-defaults/kernel.json.registerPluginnow throws. A method that previously always returned refuses a config carrying any retired key, with an ADR-0112 envelope (code: 'VALIDATION_ERROR',status: 400), before any state is stored so a refused config leaves no half-registered plugin behind. This is the door for the audience that does not parse — which is every host there is, since nothing in the tree parsesPluginHealthCheckSchemaoutside its own unit test.Tombstones rather than deletions, for #12428's reason:
PluginHealthCheckSchemais not.strict(), so a bare deletion would be a silent strip (#3733, ADR-0104) — a milder form of the very defect being retired.The pin fails on the terminal state, not on the missing call
Two pins, deliberately. The first walks the sequence and checks each step; because it does, an ablation trips one of its early assertions first. So there is a second that asserts nothing in between — it drives the failure round, the former backoff window, and
successThresholdconsecutive passes, then reads only what an operator reads:It reads the plugin's own liveness (
destroy()flips it), so — unlike a pin asserting "initwas called" — it cannot be satisfied by adding a no-opinit.Assertions changed, declared here, in the report, and on the line in the test files
Four
#11852tests and one#11955test pinned the false behaviour as the contract — each drove a plugin pastfailureThresholdand then asserteddestroyed.count === 1and statusrecovering, i.e. asserted that a restart had happened. They were true readings of a false behaviour. Every one is rewritten in place with an inline[#12032]note saying what it used to assert and why that assertion was wrong:restarts a plugin whose check THROWS…→leaves a plugin whose check THROWS alone…restarts a plugin whose check exceeds 'timeout'…→leaves a plugin that exceeds 'timeout' alone…(the timeout-report half kept verbatim)still restarts a plugin whose check RETURNS a failure→leaves a plugin whose check RETURNS a failure aloneleaves a throwing plugin alone when 'autoRestart' is false→ replaced by the refusal block (the control was vacuous once the key was gone)requires all three successes to leave the 'recovering' a restart wrote→…a 'recovering' it did not just write. Its whole point was to reachrecoveringfrom the restart path as a genuine starting state; that path is gone, so the same property is re-pinned by suspending and resuming monitoring.should validate custom health check configurationhadautoRestart: truein atoEqualfixture — the strongest single pin of the false contract.Verification
pnpm --filter @objectstack/core exec vitest run— 39 files, 987 tests passed.pnpm --filter @objectstack/spec exec vitest run— 432 files, 11480 tests passed.pnpm --filter @objectstack/spec run typecheck— green (check:test-typecheck: OK).pnpm --filter @objectstack/core build— green.pnpm lint(eslint . --no-inline-config, whole repo) — exit 0, no narrowing claimed.Gate union derived, not recalled:
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackover the real changeset → 47 gates, re-derived unchanged after the final commit. 46/47 green atea9a6c90c1, each exit code captured before any pipe.The 47th,
check:dev-prereqs, is not a finding: its scan half refuses on an unbuilt workspace ("The workspace is not built — 1 unmet precondition, not a list of problems", naming 34 app packages this diff never touches) and says so itself — "Nothing was measured". CI runs only the other half (lint.yml:3304, "self-test half only, never the scan"), which is green here:✓ check:dev-prereqs --self-test — every verdict reachable … (16 cases).Three gates first read red for the same unbuilt-prerequisite reason and were converted into real readings by building the closures rather than reported as failures:
check:doc-formula-expressions→ exit 0check:doc-security-posture→✅ 26 ObjectSchema.create example(s) in 230 marked block(s) across 237 prose file(s) … carry an os validate-clean security posturecheck:skill-examples→✅ 260 prose examples type-check across 3 surface(s)Ablation
Mutation: re-introduce the defect class —
await plugin.destroy()plushealthStatus.set(…, 'recovering')on the threshold-reached branch ofrecordFailedRound, markerOS_ABLATION_12032_DESTROY.No rebuild needed, justified by import form:
health-monitor.test.tsimports./health-monitor.js— a relative source import inside the same package, resolved by vitest tosrc/health-monitor.ts, never through the packageexportsmap todist/.Proof on disk, before any result was read:
INJECTED_MARKER_COUNT=1,INJECTED_DESTROY_CALL=1,REMOVED_SYNC_SIG_COUNT=0(the sync signature really left),git diff --numstat=14 3 packages/core/src/health-monitor.ts.Predicted in writing first, then observed:
state.alivethe monitor reported 'healthy' for a plugin that had been destroyed: expected false to be trueThe #1 miss is recorded rather than smoothed over: I wrote off
requires all three successes to leave 'unhealthy'on the grounds that itsfailureThreshold: 2needed more failing rounds than the test drives, but it drives exactly two before switching topass, so round 2 does reach the threshold. Miscounted the test's own drive, not the mechanism.Restored under
trap … EXIT INT TERM, verified:git diffonhealth-monitor.tsempty after both legs.Changeset
.changeset/plugin-auto-restart-never-reinitialised.md—@objectstack/spec: minor,@objectstack/core: minor. A BREAKING accept-set narrowing landing after the v17.0.0 cut; the lockstep launch-window convention ships it asminorand registers the prescription under protocol major 18, exactly as #12340 and #12428 did the day before in this same module.check:changeset-no-major,check:adr-0087-registrationandcheck:empty-changesetall green.Not done
content/docs/releases/untouched. No governed surface touched (docs/adr/**,.claude/**,skills/**,AGENTS.md,CLAUDE.md).Generated by Claude Code
Generated by Claude Code