diff --git a/.changeset/hot-reload-watch-placeholder-retired.md b/.changeset/hot-reload-watch-placeholder-retired.md new file mode 100644 index 0000000000..c1cc94ff5f --- /dev/null +++ b/.changeset/hot-reload-watch-placeholder-retired.md @@ -0,0 +1,86 @@ +--- +"@objectstack/spec": minor +"@objectstack/core": minor +--- + +fix(spec,core): `HotReloadManager.startWatching` refuses instead of reporting success; `HotReloadConfig.watchPatterns` retired (#12428, ADR-0049) + + + +**BREAKING** accept-set narrowing, landing after the v17.0.0 cut (the lockstep +launch-window convention ships it as `minor`; the prescription is registered +under protocol major 18 — `RETIRED_KEYS_BY_MAJOR[18]` + the D3 semantic entry +`hot-reload-watch-placeholder-retired` — where `os migrate meta` users will +look). Graded `minor` rather than `major` for the same reason #12340 was one +day earlier, in this same module. + +ADR-0049 applied one symbol over from #12340, in the same file and on the same +per-key test. The #11825 keep still stands: `HotReloadConfigSchema` and +`PluginStateSnapshotSchema` still export, and `HotReloadManager` / +`PluginHealthMonitor` are untouched apart from the two doors below. + +`HotReloadManager.startWatching` contained **no watcher**. Its whole body was a +guard plus `logger.info('File watching started', { patterns })`, above an +in-source note saying real watching "would require chokidar or similar". Where +#12340's inert fallback at least announced itself at DEBUG, this claimed +success at **INFO**: an operator who set `enabled: true` with `watchPatterns` +and read that line had been told the opposite of the truth. `watchHandles` was +only ever read, deleted, iterated and cleared and **never set**, so +`stopWatching`'s cleanup branch and the teardown loop over its keys were +structurally unreachable rather than merely untaken. `watchPatterns` therefore +had no reader that acted on it — its only two uses were log lines. + +FROM → TO: + +- `watchPatterns: ['src/**/*.ts']` → *(removed)* — delete the key. Declare your + globs wherever your own watcher reads them. +- `manager.startWatching(name)` → `manager.scheduleReload(name, reloadFn)`, + called from your own watcher's change handler. That is the debounced + integration point this class does implement, and it is unchanged. + +One-line fix: delete `watchPatterns`, and call `scheduleReload` from your own +file watcher instead of `startWatching` — nothing was ever watched, so nothing +that used to happen stops happening. File watching is the host's job in this +host-driven library; `chokidar` is already a dependency of +`@objectstack/metadata`, `@objectstack/metadata-fs` and `@objectstack/cli` — +never of `@objectstack/core` — so a host has a working model to copy. + +The retirement kit: + +- **key tombstone**, and the build is what chose it: the plain deletion was + tried first and `gen:schema` gate (a) refused it, because + `HotReloadConfigSchema` is not `.strict()` and a bare deletion would be a + silent strip (#3733, ADR-0104) — the very defect being retired, one layer + down. #12340 could take route 3 because what left there was a whole *def*; a + key leaving a *surviving* def has no such exit. So `watchPatterns` is + `retiredKey()`-tombstoned, its surface line carries `[RETIRED]`, and + `kernel/HotReloadConfig:watchPatterns` is registered by exact key in + `RETIRED_KEYS_BY_MAJOR[18]`. A key tombstone on a surviving def moves + `authorable-surface` only — the def still emits, so `api-surface` and + `json-schema.manifest` do not. +- **no D2 conversion**, deliberately: the chain walks a normalized stack, and + `HotReloadConfig` is not an authorable surface — no metadata-type binding, + stack collection or manifest embed ever carried it — so a conversion would be + a transform with no seam that ever runs. For the same reason the prescription + carries no `os migrate meta` sentence, exactly as its `stateStrategy` sibling + in this module does not. +- **runtime doors** in `@objectstack/core`, because nothing in the tree parses + `HotReloadConfigSchema` outside its own unit test, so the tombstone alone + reaches nobody: `startWatching` now throws an ADR-0112 envelope + (`code: VALIDATION_ERROR`, `status: 400`) carrying the prescription, and + `registerPlugin` refuses a leftover `watchPatterns` the same way — before the + `enabled` check, so a disabled config cannot smuggle the false declaration + through. `startWatching` is kept as a throwing door rather than deleted so + that caller meets a prescription instead of a bare `TypeError`. +- **dead code removed with a firing positive control**: `watchHandles` and both + of its unreachable readers are gone. The zero was pinned first — + `reloadTimers.set` resolves a real writer in the same file and the same scan, + while `watchHandles.set` resolves nothing anywhere. `stopWatching` keeps the + half that always did something (it cancels a pending debounced reload), and + `shutdown` is unchanged in effect: the loop it lost iterated `watchHandles` + and therefore ran zero times. +- **ENFORCE and EXPERIMENTAL were both unavailable**, which is why this is a + removal: no runtime composes `HotReloadManager`, so enforcing would build for + a caller that does not exist; and a scan of every planning doc returned zero + mentions of hot-reload file watching against 145 control hits in the same + files, so there is no roadmap for `experimental` to point at. diff --git a/content/docs/references/kernel/plugin-lifecycle-advanced.mdx b/content/docs/references/kernel/plugin-lifecycle-advanced.mdx index 3f2db79127..1b37eb6a60 100644 --- a/content/docs/references/kernel/plugin-lifecycle-advanced.mdx +++ b/content/docs/references/kernel/plugin-lifecycle-advanced.mdx @@ -41,7 +41,7 @@ const result = HotReloadConfigSchema.parse(data); | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | | **enabled** | `boolean` | optional (default: `false`) | | -| **watchPatterns** | `string[]` | optional | Glob patterns to watch for changes | +| **watchPatterns** | `never` | optional | [REMOVED] `HotReloadConfig.watchPatterns` was removed in @objectstack/spec 18 (#12428, ADR-0049 enforce-or-remove) — nothing ever read it. Its only two uses were log lines in `HotReloadManager`, and one of them announced 'File watching started' at INFO level while no watcher was ever constructed: `startWatching` held a placeholder, and `watchHandles` was read, deleted, iterated and cleared but never set. So an author could declare a glob and no file change could ever trigger a reload. Delete the key. File watching is the HOST's job in this host-driven library: run your own watcher, declare your globs wherever that watcher reads them, and call `HotReloadManager.scheduleReload(pluginName, reloadFn)` when one matches — the debounced integration point this class does implement, and which is unchanged. | | **debounceDelay** | `integer` | optional (default: `1000`) | Wait time after change detection before reload | | **preserveState** | `boolean` | optional (default: `true`) | Keep plugin state across reloads | | **stateStrategy** | `Enum<'memory' \| 'none'>` | optional (default: `"memory"`) | How to preserve state during reload | diff --git a/packages/core/examples/phase2-integration.ts b/packages/core/examples/phase2-integration.ts index 6c5d00bff4..e270a0691d 100644 --- a/packages/core/examples/phase2-integration.ts +++ b/packages/core/examples/phase2-integration.ts @@ -278,7 +278,6 @@ async function example() { // Hot reload hotReload: { enabled: true, - watchPatterns: ['plugins/my-plugin/**/*.ts'], debounceDelay: 1000, preserveState: true, stateStrategy: 'memory', diff --git a/packages/core/src/hot-reload.test.ts b/packages/core/src/hot-reload.test.ts index 4e53398c7f..b84e0baccc 100644 --- a/packages/core/src/hot-reload.test.ts +++ b/packages/core/src/hot-reload.test.ts @@ -328,3 +328,136 @@ describe('[#12340] stateStrategy refusal', () => { }); } }); + + +// ── [#12428] The watching placeholder refuses instead of reporting success ─── +// +// Before this card `startWatching` contained NO watcher: a guard plus +// `logger.info('File watching started', { patterns })` above an in-source note +// saying real watching "would require chokidar or similar". An operator who set +// `enabled: true` with `watchPatterns` and read that INFO line had been told the +// opposite of the truth. `watchHandles` was only ever read, deleted, iterated +// and cleared and NEVER set, so `stopWatching`'s cleanup branch and the teardown +// loop over its keys were structurally unreachable, not merely untaken. +// +// The last two tests are the behaviour-PRESERVATION pins for those removals: +// what left was unreachable, and what stayed still works. +describe('[#12428] startWatching refusal and the watch-handle removal', () => { + const liveConfig = (overrides: Record = {}): HotReloadConfigParsed => + ({ + enabled: true, + debounceDelay: 1000, + preserveState: false, + stateStrategy: 'memory', + shutdownTimeout: 1000, + ...overrides, + }) as unknown as HotReloadConfigParsed; + + let mgr: HotReloadManager; + beforeEach(() => { + mgr = new HotReloadManager(createRecordingLogger([])); + }); + + it('refuses startWatching with an ADR-0112 envelope and the prescription', () => { + mgr.registerPlugin('p', liveConfig()); + + let caught: (Error & { code?: string; status?: number }) | undefined; + try { + mgr.startWatching('p'); + } catch (e) { + caught = e as Error & { code?: string; status?: number }; + } + + // The envelope, not merely "it threw": a bare toThrow() would stay green + // against any unrelated failure on this path. + expect(caught, 'startWatching must be refused').toBeDefined(); + expect(caught?.code).toBe('VALIDATION_ERROR'); + expect(caught?.status).toBe(400); + + // The prescription's load-bearing facts, by CONTENT — this message is the + // whole migration document for whoever hits it. + const m = caught?.message ?? ''; + expect(m).toContain('#12428'); + expect(m).toContain('ADR-0049'); + expect(m).toContain('never watched'); + expect(m).toContain('scheduleReload'); + expect(m).toContain('p'); // locates the offending plugin + }); + + it('refuses startWatching for an UNREGISTERED plugin too', () => { + // The old body early-returned when the plugin was unknown or disabled, so + // the lie was conditional. The refusal must not be: the method never + // worked for anyone, in any state. + expect(() => mgr.startWatching('never-registered')).toThrow(/#12428/); + }); + + it('refuses a leftover watchPatterns at registration', () => { + // The schema is not .strict(), so zod would STRIP this key on any parse + // path — a clean parse and a setting that never takes effect. Route 3 left + // no parse-time prescription (nothing parses the schema), so THIS is the + // door that keeps the removal honest for the audience that exists. + let caught: (Error & { code?: string; status?: number }) | undefined; + try { + mgr.registerPlugin('p', liveConfig({ watchPatterns: ['src/**/*.ts'] })); + } catch (e) { + caught = e as Error & { code?: string; status?: number }; + } + expect(caught).toBeDefined(); + expect(caught?.code).toBe('VALIDATION_ERROR'); + expect(caught?.status).toBe(400); + expect(caught?.message).toContain('watchPatterns'); + expect(caught?.message).toContain('#12428'); + expect(caught?.message).toContain('nothing ever read it'); + }); + + it('refuses watchPatterns even when hot reload is disabled', () => { + // The door must not depend on `enabled` — a false declaration is false + // whether or not the feature is switched on. + expect(() => + mgr.registerPlugin('p', liveConfig({ enabled: false, watchPatterns: ['a/**'] })) + ).toThrow(/#12428/); + }); + + it('still registers a config that does not carry the retired key', () => { + // Anti-vacuity for the door: the refusals above must be about the key, not + // about registration having broken. + expect(() => mgr.registerPlugin('p', liveConfig())).not.toThrow(); + }); + + it('stopWatching still cancels a pending debounced reload', () => { + // Behaviour preservation for the `watchHandles` removal. What left was the + // unreachable cleanup branch; the half that always did something — clearing + // the debounce timer armed by `scheduleReload` — is untouched. + vi.useFakeTimers(); + try { + mgr.registerPlugin('p', liveConfig()); + let ran = 0; + mgr.scheduleReload('p', async () => { ran++; }); + + mgr.stopWatching('p'); + vi.advanceTimersByTime(5000); + expect(ran, 'the scheduled reload must have been cancelled').toBe(0); + } finally { + vi.useRealTimers(); + } + }); + + it('shutdown still clears pending timers without the dead teardown loop', () => { + // The removed loop iterated `watchHandles.keys()` and therefore ran zero + // times; every timer it could have reached is cleared by the + // `reloadTimers` loop that follows it. This pins that equivalence. + vi.useFakeTimers(); + try { + mgr.registerPlugin('p', liveConfig()); + let ran = 0; + mgr.scheduleReload('p', async () => { ran++; }); + + mgr.shutdown(); + vi.advanceTimersByTime(5000); + expect(ran, 'shutdown must leave no pending reload behind').toBe(0); + expect(vi.getTimerCount()).toBe(0); + } finally { + vi.useRealTimers(); + } + }); +}); diff --git a/packages/core/src/hot-reload.ts b/packages/core/src/hot-reload.ts index 81bd5015a9..59e3017e9d 100644 --- a/packages/core/src/hot-reload.ts +++ b/packages/core/src/hot-reload.ts @@ -60,7 +60,7 @@ const RETIRED_STATE_STRATEGY_GUIDANCE = * threw". `VALIDATION_ERROR` is the standard catalog's generic * argument-validation code, following `metadata-service-contract.ts`. */ -function stateStrategyRefusal(message: string): Error & { code: string; status: number } { +function hotReloadRefusal(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; @@ -82,7 +82,7 @@ function assertHonouredStateStrategy(pluginName: string, strategy: unknown): voi } const shown = typeof strategy === 'string' ? `'${strategy}'` : String(strategy); const retired = strategy === 'disk' || strategy === 'distributed'; - throw stateStrategyRefusal( + throw hotReloadRefusal( `[HotReload] Plugin '${pluginName}': unsupported stateStrategy ${shown}. ` + `Honoured values are ${HONOURED_STATE_STRATEGIES.map((v) => `'${v}'`).join(' and ')}. ` + (retired @@ -92,29 +92,56 @@ function assertHonouredStateStrategy(pluginName: string, strategy: unknown): voi } /** - * Refuse a `distributedConfig` left over from before #12340. + * Keys removed from `HotReloadConfig` that a host may still be passing. * - * `HotReloadConfigSchema` is not `.strict()`, so zod would silently STRIP this - * key on the parse paths that exist — a clean parse and a setting that never - * takes effect, which is the exact failure the authorable-surface gate names. - * The key was removed rather than tombstoned (route 3: nothing in the tree - * parses this schema, so a parse-time prescription reaches nobody), and this - * is the door that makes that route honest for the audience that DOES exist — - * a host handing the object straight to the class. + * `HotReloadConfigSchema` is not `.strict()`, so zod would silently STRIP each + * of these on the parse paths that exist — a clean parse and a setting that + * never takes effect, which is the exact failure the authorable-surface gate + * names. Both keys were removed rather than tombstoned (route 3: nothing in + * the tree parses this schema, so a parse-time prescription reaches nobody), + * and this table is the door that makes that route honest for the audience + * that DOES exist — a host handing the object straight to the class. + * + * Each entry is the guidance clause; the `[HotReload] Plugin '': ` + * prefix is added at throw time. #12340's `distributedConfig` message is + * carried across byte-for-byte — its pins assert content, and this + * generalisation must not move them. */ -function assertNoRetiredDistributedConfig(pluginName: string, config: object): void { - if (!Object.prototype.hasOwnProperty.call(config, 'distributedConfig')) { - return; - } - throw stateStrategyRefusal( - `[HotReload] Plugin '${pluginName}': 'distributedConfig' was removed from ` +const RETIRED_HOT_RELOAD_KEYS: ReadonlyArray = [ + [ + 'distributedConfig', + "'distributedConfig' was removed from " + 'HotReloadConfig in @objectstack/spec 18 (#12340, ADR-0049 ' + 'enforce-or-remove) — nothing ever read it. A provider, endpoints, a key ' + 'prefix, a TTL and a replication factor could all be declared and no ' + "connection was ever opened. It left with the stateStrategy: 'distributed' " + 'value it was documented as being required for. Delete the key; there is no ' - + 'in-tree replacement for distributed plugin state — persist it in the host.' - ); + + 'in-tree replacement for distributed plugin state — persist it in the host.', + ], + [ + 'watchPatterns', + "'watchPatterns' was removed from HotReloadConfig in @objectstack/spec 18 " + + '(#12428, ADR-0049 enforce-or-remove) — nothing ever read it. Its only two ' + + 'uses were log lines: no watcher was ever constructed from it, so an author ' + + 'could declare a glob and no file change ever triggered a reload. File ' + + 'watching is the HOST\'s job in this host-driven library. Delete the key, ' + + 'declare your globs wherever your own watcher reads them, and call ' + + '`HotReloadManager.scheduleReload(pluginName, reloadFn)` when one matches — ' + + 'that is the debounced integration point this class does implement.', + ], +]; + +/** + * Refuse a key this library removed, at the moment the host hands the config + * over. First match wins; the order is the order they were retired. + */ +function assertNoRetiredKeys(pluginName: string, config: object): void { + for (const [key, guidance] of RETIRED_HOT_RELOAD_KEYS) { + if (!Object.prototype.hasOwnProperty.call(config, key)) { + continue; + } + throw hotReloadRefusal(`[HotReload] Plugin '${pluginName}': ${guidance}`); + } } /** @@ -242,7 +269,6 @@ export class HotReloadManager { private logger: ObjectLogger; private stateManager: PluginStateManager; private reloadConfigs = new Map(); - private watchHandles = new Map(); private reloadTimers = new Map(); constructor(logger: ObjectLogger) { @@ -260,7 +286,7 @@ export class HotReloadManager { // would let the false declaration through on exactly the configs nobody // is watching. assertHonouredStateStrategy(pluginName, config.stateStrategy); - assertNoRetiredDistributedConfig(pluginName, config); + assertNoRetiredKeys(pluginName, config); if (!config.enabled) { this.logger.debug('Hot reload disabled for plugin', { plugin: pluginName }); @@ -270,40 +296,57 @@ export class HotReloadManager { this.reloadConfigs.set(pluginName, config); this.logger.info('Plugin registered for hot reload', { plugin: pluginName, - watchPatterns: config.watchPatterns, stateStrategy: config.stateStrategy }); } /** - * Start watching for changes (requires file system integration) + * Refuse the file-watching call this class never implemented (#12428). + * + * The body used to be a guard plus `logger.info('File watching started')` + * over an in-source note saying real watching "would require chokidar or + * similar". Nothing was ever watched, so an operator who set + * `enabled: true` and read that line at INFO had been told the opposite of + * the truth — positive confirmation of a capability that did not exist. + * ADR-0049 leaves three states and this surface qualified for none of the + * other two: no runtime composes this class, so ENFORCE would build for a + * caller that does not exist, and no roadmap entry anywhere claims the + * feature, so EXPERIMENTAL would be a promise nobody made. + * + * Kept as a throwing door rather than deleted: removing the method leaves a + * JavaScript host a bare `TypeError: not a function` with no prescription, + * and this is the one place a caller of the old placeholder is guaranteed + * to arrive. The refusal carries an ADR-0112 envelope so it can be asserted + * rather than merely caught. */ - startWatching(pluginName: string): void { - const config = this.reloadConfigs.get(pluginName); - if (!config || !config.enabled) { - return; - } - - // Note: Actual file watching would require chokidar or similar - // This is a placeholder for the integration point - this.logger.info('File watching started', { - plugin: pluginName, - patterns: config.watchPatterns - }); + startWatching(pluginName: string): never { + throw hotReloadRefusal( + `[HotReload] Plugin '${pluginName}': startWatching() never watched ` + + 'anything and was removed in @objectstack/core 18 (#12428, ADR-0049 ' + + "enforce-or-remove). It logged 'File watching started' at info level " + + 'while no watcher was ever constructed, so no file change could ever ' + + 'trigger a reload. File watching is the HOST\'s job in this ' + + 'host-driven library: run your own watcher and call ' + + '`HotReloadManager.scheduleReload(pluginName, reloadFn)` when a file ' + + 'changes — that is the debounced integration point this class does ' + + 'implement. `HotReloadConfig.watchPatterns` was removed in ' + + '@objectstack/spec 18 for the same reason; declare your globs where ' + + 'your watcher reads them.' + ); } /** - * Stop watching for changes + * Cancel a pending debounced reload for a plugin. + * + * The name is historical (#12428). This never stopped a watcher, because + * nothing in this class ever started one: its `watchHandles` cleanup branch + * read a Map that had no writer anywhere in the tree, so the branch was + * structurally unreachable rather than merely untaken, and it left with + * `startWatching`'s placeholder. What survives is the half that always did + * something — the debounce timer armed by `scheduleReload` is cleared, so a + * reload that was scheduled but has not fired yet is cancelled. */ stopWatching(pluginName: string): void { - const handle = this.watchHandles.get(pluginName); - if (handle) { - // Stop watching (would call chokidar close()) - this.watchHandles.delete(pluginName); - this.logger.info('File watching stopped', { plugin: pluginName }); - } - - // Clear any pending reload timers const timer = this.reloadTimers.get(pluginName); if (timer) { clearTimeout(timer); @@ -489,18 +532,12 @@ export class HotReloadManager { * Shutdown hot reload manager */ shutdown(): void { - // Stop all watching - for (const pluginName of this.watchHandles.keys()) { - this.stopWatching(pluginName); - } - // Clear all timers for (const timer of this.reloadTimers.values()) { clearTimeout(timer); } this.reloadConfigs.clear(); - this.watchHandles.clear(); this.reloadTimers.clear(); this.stateManager.shutdown(); diff --git a/packages/spec/authorable-surface/kernel.json b/packages/spec/authorable-surface/kernel.json index 85978d1ed7..664b19c751 100644 --- a/packages/spec/authorable-surface/kernel.json +++ b/packages/spec/authorable-surface/kernel.json @@ -228,7 +228,7 @@ "kernel/HotReloadConfig:preserveState", "kernel/HotReloadConfig:shutdownTimeout", "kernel/HotReloadConfig:stateStrategy", - "kernel/HotReloadConfig:watchPatterns", + "kernel/HotReloadConfig:watchPatterns [RETIRED]", "kernel/InstallPackageRequest:enableOnInstall", "kernel/InstallPackageRequest:manifest", "kernel/InstallPackageRequest:platformVersion", diff --git a/packages/spec/src/kernel/plugin-lifecycle-advanced.test.ts b/packages/spec/src/kernel/plugin-lifecycle-advanced.test.ts index c8bf13b821..2ba42479d6 100644 --- a/packages/spec/src/kernel/plugin-lifecycle-advanced.test.ts +++ b/packages/spec/src/kernel/plugin-lifecycle-advanced.test.ts @@ -121,7 +121,10 @@ describe('Plugin Lifecycle Advanced Schemas', () => { it('should validate custom hot reload configuration', () => { const config = { enabled: true, - watchPatterns: ['src/**/*.ts', 'config/**/*.json'], + // [#12428] `watchPatterns` REMOVED from this fixture — declared, not a + // quiet edit. It used to be listed here and asserted via toEqual below, + // an assertion that passed precisely BECAUSE the key parsed and did + // nothing. The key's departure is pinned as a STRIP in its own test. debounceDelay: 2000, preserveState: false, stateStrategy: 'memory' as const, @@ -185,6 +188,31 @@ describe('Plugin Lifecycle Advanced Schemas', () => { } as Record); expect(result).not.toHaveProperty('distributedConfig'); }); + + it('refuses watchPatterns with the retirement prescription (#12428)', () => { + // Unlike the distributedConfig pin above, this one asserts a REFUSAL, not + // a strip: `watchPatterns` is `retiredKey()`-tombstoned. A bare deletion + // was tried first and `gen:schema` gate (a) refused it — this object is + // not `.strict()`, so deleting the key would be a silent strip (#3733, + // ADR-0104), which is the very defect being retired. + const result = HotReloadConfigSchema.safeParse({ + enabled: true, + watchPatterns: ['src/**/*.ts'], + } as Record); + expect(result.success, 'watchPatterns 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 byte string. + const message = result.success ? '' : result.error.issues[0]?.message ?? ''; + expect(message).toContain('was removed'); + expect(message).toContain('#12428'); + expect(message).toContain('ADR-0049'); + expect(message).toContain('scheduleReload'); + + // Anti-vacuity: the surrounding keep still parses, so the refusal above + // is about this key and not about the schema having broken. + expect(HotReloadConfigSchema.safeParse({ enabled: true }).success).toBe(true); + }); }); describe('PluginStateSnapshotSchema', () => { diff --git a/packages/spec/src/kernel/plugin-lifecycle-advanced.zod.ts b/packages/spec/src/kernel/plugin-lifecycle-advanced.zod.ts index ae1e3dfaca..1d54c2f20b 100644 --- a/packages/spec/src/kernel/plugin-lifecycle-advanced.zod.ts +++ b/packages/spec/src/kernel/plugin-lifecycle-advanced.zod.ts @@ -22,6 +22,7 @@ import { z } from 'zod'; * Represents the current operational state of a plugin */ import { lazySchema } from '../shared/lazy-schema'; +import { retiredKey } from '../shared/retired-key'; export const PluginHealthStatusSchema = lazySchema(() => z.enum([ 'healthy', // Plugin is operating normally 'degraded', // Plugin is operational but with reduced functionality @@ -167,6 +168,34 @@ const HOT_RELOAD_STATE_STRATEGY_RETIRED = + 'persistence returns only via the ENFORCE route of ADR-0049: the ' + 'implementation first, the declaration with it.'; +/** + * Prescription for the watch-placeholder key retired in 18 (#12428). + * + * Carries NO `os migrate meta --from 17` sentence, for exactly the reason + * `HOT_RELOAD_STATE_STRATEGY_RETIRED` above does not: that command replays the + * conversion chain over authored METADATA SOURCES, and `HotReloadConfig` is + * not an authorable surface — it is a library parameter a host passes to + * `HotReloadManager` in TypeScript (the #4914 / #11825 keep). No authored + * document has ever been able to carry `watchPatterns`, 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. + */ +const HOT_RELOAD_WATCH_PATTERNS_RETIRED = + '`HotReloadConfig.watchPatterns` was removed in @objectstack/spec 18 ' + + '(#12428, ADR-0049 enforce-or-remove) — nothing ever read it. Its only two ' + + 'uses were log lines in `HotReloadManager`, and one of them announced ' + + "'File watching started' at INFO level while no watcher was ever " + + 'constructed: `startWatching` held a placeholder, and `watchHandles` was ' + + 'read, deleted, iterated and cleared but never set. So an author could ' + + 'declare a glob and no file change could ever trigger a reload. Delete the ' + + 'key. File watching is the HOST\'s job in this host-driven library: run ' + + 'your own watcher, declare your globs wherever that watcher reads them, ' + + 'and call `HotReloadManager.scheduleReload(pluginName, reloadFn)` when one ' + + 'matches — the debounced integration point this class does implement, and ' + + 'which is unchanged.'; + /** * Hot Reload Configuration * Controls how plugins handle live updates @@ -178,11 +207,17 @@ export const HotReloadConfigSchema = lazySchema(() => z.object({ enabled: z.boolean().default(false), /** - * Watch file patterns for auto-reload + * REMOVED in 18 (#12428) — 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 the very defect being retired, one layer down. 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. */ - watchPatterns: z.array(z.string()).optional() - .describe('Glob patterns to watch for changes'), - + watchPatterns: retiredKey(HOT_RELOAD_WATCH_PATTERNS_RETIRED), + /** * Debounce delay before reloading (milliseconds) */ @@ -306,6 +341,57 @@ export const HotReloadConfigSchema = lazySchema(() => z.object({ // `plugin-lifecycle-advanced-retirement.test.ts` moves with it, deliberately // and in the same commit — never as a quiet edit to make a red pin green. // +// ── [#12428] AMENDED 2026-08-26: the placeholder that reported success ────── +// +// The keep is STILL intact — `HotReloadConfigSchema` and `HotReloadManager` +// stay. `watchPatterns` is REMOVED, on the same per-key test #12340 applied to +// `distributedConfig` and measured the same way (positive control fired: +// `reloadTimers.set` resolves a real writer in `core/src/hot-reload.ts`, so +// the scan sees writers; `watchHandles.set` resolves nothing anywhere). +// +// What was measured: `HotReloadManager.startWatching` contained NO watcher — +// a guard plus `logger.info('File watching started', { patterns })` over an +// in-source note saying real watching "would require chokidar or similar". +// `watchHandles` was only ever read, deleted, iterated and cleared and NEVER +// set, so `stopWatching`'s cleanup branch and the teardown loop over its keys +// were structurally UNREACHABLE, not merely untaken. So `watchPatterns` had +// no reader that ACTED on it: an author could declare a glob and no file +// change could ever trigger a reload. +// +// Worse than #12340's silence, in one specific way: that fallback at least +// announced itself at DEBUG. This one said "File watching started" at INFO — +// positive confirmation of a capability that did not exist, which an operator +// (or an AI author, ADR-0033) reads as proof and stops looking. +// +// Why REMOVE and not the other two ADR-0049 states: ENFORCE would build for a +// caller that does not exist (no runtime composes `HotReloadManager`; only +// its own unit test and `core/examples/phase2-integration.ts` construct it) — +// the same fact that decided #12340's route. EXPERIMENTAL requires a +// roadmap, and a scan of every planning doc found zero mentions of hot-reload +// file watching against 145 control hits in the same files. Real watching +// already lives where it is implemented: `chokidar` is a dependency of +// `@objectstack/metadata`, `@objectstack/metadata-fs` and `@objectstack/cli`, +// never of `@objectstack/core`. +// +// Route: TOMBSTONE, not #12340's route 3, and the build is what decided it. +// The plain deletion was tried first and `gen:schema` gate (a) refused it — +// 'authorable key(s) disappeared from the contract' — because this object is +// not `.strict()` and a bare deletion is a SILENT STRIP (#3733, ADR-0104). +// #12340 could take route 3 because what left there was a whole DEF; a key +// leaving a SURVIVING def has no such exit. So `watchPatterns` is +// `retiredKey()`-tombstoned and registered by exact key in +// RETIRED_KEYS_BY_MAJOR[18], its surface line carrying `[RETIRED]` rather +// than disappearing. A key tombstone on a surviving def moves +// `authorable-surface` only — the def still emits, so `api-surface` and +// `json-schema.manifest` do not. +// +// The tombstone answers the parse; the registration-time refusal in +// `HotReloadManager` answers the audience that does NOT parse — a host +// handing the object straight to the class, which is every host there is, +// since nothing in the tree parses `HotReloadConfigSchema` outside its own +// unit test. The D3 semantic entry +// `hot-reload-watch-placeholder-retired` records the reasoning. +// // Route 3 (no tombstone, no conversion): with no carrier key and no authored // document there is nothing to tombstone and no seam for a D2 conversion — // `RETIRED_DEFS_BY_MAJOR[18]` (`kernel/AdvancedPluginLifecycleConfig`, diff --git a/packages/spec/src/migrations/entries/retired-keys/18.kernel__HotReloadConfig__watchPatterns.ts b/packages/spec/src/migrations/entries/retired-keys/18.kernel__HotReloadConfig__watchPatterns.ts new file mode 100644 index 0000000000..0f2dfd6f34 --- /dev/null +++ b/packages/spec/src/migrations/entries/retired-keys/18.kernel__HotReloadConfig__watchPatterns.ts @@ -0,0 +1,40 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +// #12428 — ADR-0049 enforce-or-remove, one symbol over from #12340 (PR #12425) +// in the same file and on the same per-key test. `HotReloadManager.startWatching` +// contained NO watcher: a guard plus `logger.info('File watching started', +// { patterns })` above an in-source note saying real watching "would require +// chokidar or similar / This is a placeholder for the integration point". +// `watchHandles` was only ever read, deleted, iterated and cleared and NEVER +// set, so `stopWatching`'s cleanup branch and the teardown loop over its keys +// were structurally UNREACHABLE rather than merely untaken — measured with a +// firing positive control (`reloadTimers.set` resolves a real writer in the +// same file and the same scan; `watchHandles.set` resolves nothing anywhere). +// `watchPatterns` therefore had no reader that ACTED on it, and #12340's +// silence was at least at DEBUG level where this one claimed success at INFO. +// +// Neither of ADR-0049's other two states was available: ENFORCE would build for +// a caller that does not exist (no runtime composes `HotReloadManager` — only +// its own unit test and `core/examples/phase2-integration.ts`), and +// EXPERIMENTAL requires a roadmap, where a scan of every planning doc returned +// ZERO hits for hot-reload file watching against 145 control hits in the same +// files. Real watching lives where it is implemented: `chokidar` is a +// dependency of `@objectstack/metadata`, `@objectstack/metadata-fs` and +// `@objectstack/cli`, never of `@objectstack/core`. +// +// Registered under 18, not 17: v17.0.0 was cut before this landed, so the +// removal ships on the 17.x line (launch-window convention: accept-set +// narrowings ride minor releases) and the prescription lives at the major +// boundary where `migrate meta` users look — the same grading #12340 used one +// day earlier in this module. +// +// Tombstoned with `retiredKey()` in `HotReloadConfigSchema` (the surface +// baseline line carries `[RETIRED]`). Deliberately NO D2 conversion: the chain +// walks a normalized STACK, and `HotReloadConfig` is not an authorable surface +// — no metadata-type binding, stack collection or manifest embed ever carried +// it, and nothing in the tree parses `HotReloadConfigSchema` outside its own +// unit test — so a conversion would be a transform with no seam that ever +// runs. The D3 semantic entry `hot-reload-watch-placeholder-retired` is the +// declaration, and the registration-time refusal in +// `HotReloadManager.registerPlugin` is the door for the audience that exists. +export const entry = 'kernel/HotReloadConfig:watchPatterns'; diff --git a/packages/spec/src/migrations/entries/semantic/18.hot-reload-watch-placeholder-retired.ts b/packages/spec/src/migrations/entries/semantic/18.hot-reload-watch-placeholder-retired.ts new file mode 100644 index 0000000000..c840c6f773 --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.hot-reload-watch-placeholder-retired.ts @@ -0,0 +1,81 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +export const entry: SemanticMigration = { + id: 'hot-reload-watch-placeholder-retired', + surface: + '`HotReloadConfig.watchPatterns`, and the `HotReloadManager.startWatching` ' + + 'placeholder that read it', + replacement: + 'Run your own watcher and call ' + + '`HotReloadManager.scheduleReload(pluginName, reloadFn)` when a file ' + + 'changes — that is the debounced integration point this class actually ' + + 'implements, and it is unchanged. Declare your globs wherever your ' + + 'watcher reads them; there is no in-tree replacement for the key, ' + + 'because file watching is the HOST\'s job in this host-driven library. ' + + 'The platform already depends on `chokidar` in `@objectstack/metadata`, ' + + '`@objectstack/metadata-fs` and `@objectstack/cli` — never in ' + + '`@objectstack/core` — so a host has a working model to copy.', + reason: + 'ADR-0049 enforce-or-remove, applied one symbol over from #12340 in the ' + + 'same file and on the same per-key test. `HotReloadManager.startWatching` ' + + 'contained NO watcher: its whole body was a guard plus ' + + "`logger.info('File watching started', { patterns })` above an in-source " + + 'note saying real watching "would require chokidar or similar / This is a ' + + 'placeholder for the integration point". `watchHandles` was only ever ' + + 'read, deleted, iterated and cleared and NEVER set, so `stopWatching`\'s ' + + 'cleanup branch and the teardown loop over its keys were structurally ' + + 'UNREACHABLE rather than merely untaken (measured with a firing positive ' + + 'control: `reloadTimers.set` resolves a real writer in the same file and ' + + 'the same scan; `watchHandles.set` resolves nothing anywhere). So ' + + '`watchPatterns` had no reader that ACTED on it — its only two uses were ' + + 'log lines — and an author could declare a glob while no file change ' + + 'could ever trigger a reload. This is the #3950 shape with the volume ' + + 'turned up: #12340\'s inert fallback at least announced itself at DEBUG, ' + + 'whereas this said "File watching started" at INFO — positive ' + + 'confirmation of a capability that did not exist, which an operator, or ' + + 'an AI author (ADR-0033), reads as proof and stops looking. Neither of ' + + 'the other two ADR-0049 states was available: ENFORCE would build for a ' + + 'caller that does not exist (no runtime composes `HotReloadManager` — ' + + 'only its own unit test and `core/examples/phase2-integration.ts` ' + + 'construct it, the same fact that decided #12340\'s route), and ' + + 'EXPERIMENTAL requires a roadmap, where a scan of every planning doc ' + + 'returned ZERO mentions of hot-reload file watching against 145 control ' + + 'hits in the same files. Route 3 again: `HotReloadConfig` is not an ' + + 'authorable surface — no metadata-type binding, stack collection or ' + + 'manifest embed ever carried it, and nothing in the tree parses ' + + '`HotReloadConfigSchema` outside its own unit test — so there is no ' + + 'authored document to rewrite and nobody who could receive a parse-time ' + + 'prescription, so there is no D2 conversion either — it would be a ' + + 'transform with no seam that ever runs. The key is TOMBSTONED rather ' + + 'than deleted, and the BUILD is what decided that: the plain deletion ' + + 'was tried first and `gen:schema` gate (a) refused it, because ' + + '`HotReloadConfigSchema` is not `.strict()` and a bare deletion would ' + + 'be a silent strip (#3733, ADR-0104) — the very defect being retired, ' + + 'one layer down. #12340 could take route 3 because what left there was ' + + 'a whole DEF; a key leaving a SURVIVING def has no such exit. This ' + + 'entry IS the declaration.', + acceptanceCriteria: + 'No host passes `watchPatterns` to `HotReloadManager.registerPlugin`, and ' + + 'no host calls `HotReloadManager.startWatching`. TypeScript hosts cannot ' + + 'do either: `watchPatterns` is typed `never` by the tombstone, and ' + + '`startWatching` returns `never`. 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 for ' + + 'a leftover `watchPatterns` BEFORE the `enabled` check so a disabled ' + + 'config cannot smuggle the false declaration through, and thrown ' + + 'unconditionally from `startWatching` so the placeholder can no longer ' + + 'report success. `startWatching` is kept as a throwing door rather than ' + + 'deleted precisely so that caller meets a prescription instead of a bare ' + + '`TypeError: not a function`. Runtime reload behaviour is UNCHANGED for ' + + 'every config that worked: nothing was ever watched, so nothing that ' + + 'used to happen stops happening — `registerPlugin`, `scheduleReload`, ' + + '`reloadPlugin` and state preservation are untouched, and ' + + '`stopWatching` keeps the half that always did something (it cancels a ' + + 'pending debounced reload; its unreachable `watchHandles` branch left ' + + 'with the placeholder). The #11825 keep still stands: ' + + '`HotReloadConfigSchema` and `PluginStateSnapshotSchema` still export ' + + 'from `./kernel`, and `HotReloadManager` / `PluginHealthMonitor` still ' + + 'export from `@objectstack/core` with their tests green.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 76794498f9..c34052d9bf 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -6181,6 +6181,83 @@ const step18: MigrationStep = { + '`./kernel`, and `HotReloadManager` / `PluginHealthMonitor` still export ' + 'from `@objectstack/core` with their tests green.', }, + { + id: 'hot-reload-watch-placeholder-retired', + surface: + '`HotReloadConfig.watchPatterns`, and the `HotReloadManager.startWatching` ' + + 'placeholder that read it', + replacement: + 'Run your own watcher and call ' + + '`HotReloadManager.scheduleReload(pluginName, reloadFn)` when a file ' + + 'changes — that is the debounced integration point this class actually ' + + 'implements, and it is unchanged. Declare your globs wherever your ' + + 'watcher reads them; there is no in-tree replacement for the key, ' + + 'because file watching is the HOST\'s job in this host-driven library. ' + + 'The platform already depends on `chokidar` in `@objectstack/metadata`, ' + + '`@objectstack/metadata-fs` and `@objectstack/cli` — never in ' + + '`@objectstack/core` — so a host has a working model to copy.', + reason: + 'ADR-0049 enforce-or-remove, applied one symbol over from #12340 in the ' + + 'same file and on the same per-key test. `HotReloadManager.startWatching` ' + + 'contained NO watcher: its whole body was a guard plus ' + + "`logger.info('File watching started', { patterns })` above an in-source " + + 'note saying real watching "would require chokidar or similar / This is a ' + + 'placeholder for the integration point". `watchHandles` was only ever ' + + 'read, deleted, iterated and cleared and NEVER set, so `stopWatching`\'s ' + + 'cleanup branch and the teardown loop over its keys were structurally ' + + 'UNREACHABLE rather than merely untaken (measured with a firing positive ' + + 'control: `reloadTimers.set` resolves a real writer in the same file and ' + + 'the same scan; `watchHandles.set` resolves nothing anywhere). So ' + + '`watchPatterns` had no reader that ACTED on it — its only two uses were ' + + 'log lines — and an author could declare a glob while no file change ' + + 'could ever trigger a reload. This is the #3950 shape with the volume ' + + 'turned up: #12340\'s inert fallback at least announced itself at DEBUG, ' + + 'whereas this said "File watching started" at INFO — positive ' + + 'confirmation of a capability that did not exist, which an operator, or ' + + 'an AI author (ADR-0033), reads as proof and stops looking. Neither of ' + + 'the other two ADR-0049 states was available: ENFORCE would build for a ' + + 'caller that does not exist (no runtime composes `HotReloadManager` — ' + + 'only its own unit test and `core/examples/phase2-integration.ts` ' + + 'construct it, the same fact that decided #12340\'s route), and ' + + 'EXPERIMENTAL requires a roadmap, where a scan of every planning doc ' + + 'returned ZERO mentions of hot-reload file watching against 145 control ' + + 'hits in the same files. Route 3 again: `HotReloadConfig` is not an ' + + 'authorable surface — no metadata-type binding, stack collection or ' + + 'manifest embed ever carried it, and nothing in the tree parses ' + + '`HotReloadConfigSchema` outside its own unit test — so there is no ' + + 'authored document to rewrite and nobody who could receive a parse-time ' + + 'prescription, so there is no D2 conversion either — it would be a ' + + 'transform with no seam that ever runs. The key is TOMBSTONED rather ' + + 'than deleted, and the BUILD is what decided that: the plain deletion ' + + 'was tried first and `gen:schema` gate (a) refused it, because ' + + '`HotReloadConfigSchema` is not `.strict()` and a bare deletion would ' + + 'be a silent strip (#3733, ADR-0104) — the very defect being retired, ' + + 'one layer down. #12340 could take route 3 because what left there was ' + + 'a whole DEF; a key leaving a SURVIVING def has no such exit. This ' + + 'entry IS the declaration.', + acceptanceCriteria: + 'No host passes `watchPatterns` to `HotReloadManager.registerPlugin`, and ' + + 'no host calls `HotReloadManager.startWatching`. TypeScript hosts cannot ' + + 'do either: `watchPatterns` is typed `never` by the tombstone, and ' + + '`startWatching` returns `never`. 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 for ' + + 'a leftover `watchPatterns` BEFORE the `enabled` check so a disabled ' + + 'config cannot smuggle the false declaration through, and thrown ' + + 'unconditionally from `startWatching` so the placeholder can no longer ' + + 'report success. `startWatching` is kept as a throwing door rather than ' + + 'deleted precisely so that caller meets a prescription instead of a bare ' + + '`TypeError: not a function`. Runtime reload behaviour is UNCHANGED for ' + + 'every config that worked: nothing was ever watched, so nothing that ' + + 'used to happen stops happening — `registerPlugin`, `scheduleReload`, ' + + '`reloadPlugin` and state preservation are untouched, and ' + + '`stopWatching` keeps the half that always did something (it cancels a ' + + 'pending debounced reload; its unreachable `watchHandles` branch left ' + + 'with the placeholder). The #11825 keep still stands: ' + + '`HotReloadConfigSchema` and `PluginStateSnapshotSchema` still export ' + + 'from `./kernel`, and `HotReloadManager` / `PluginHealthMonitor` still ' + + 'export from `@objectstack/core` with their tests green.', + }, { id: 'identity-api-key-schema-retired', surface: @@ -7318,6 +7395,44 @@ export const RETIRED_KEYS_BY_MAJOR: Readonly> // conversion `metric-filters-removed`, which strips the key from every metric // in `analyticsCubes[].measures`. 'data/Metric:filters', + // #12428 — ADR-0049 enforce-or-remove, one symbol over from #12340 (PR #12425) + // in the same file and on the same per-key test. `HotReloadManager.startWatching` + // contained NO watcher: a guard plus `logger.info('File watching started', + // { patterns })` above an in-source note saying real watching "would require + // chokidar or similar / This is a placeholder for the integration point". + // `watchHandles` was only ever read, deleted, iterated and cleared and NEVER + // set, so `stopWatching`'s cleanup branch and the teardown loop over its keys + // were structurally UNREACHABLE rather than merely untaken — measured with a + // firing positive control (`reloadTimers.set` resolves a real writer in the + // same file and the same scan; `watchHandles.set` resolves nothing anywhere). + // `watchPatterns` therefore had no reader that ACTED on it, and #12340's + // silence was at least at DEBUG level where this one claimed success at INFO. + // + // Neither of ADR-0049's other two states was available: ENFORCE would build for + // a caller that does not exist (no runtime composes `HotReloadManager` — only + // its own unit test and `core/examples/phase2-integration.ts`), and + // EXPERIMENTAL requires a roadmap, where a scan of every planning doc returned + // ZERO hits for hot-reload file watching against 145 control hits in the same + // files. Real watching lives where it is implemented: `chokidar` is a + // dependency of `@objectstack/metadata`, `@objectstack/metadata-fs` and + // `@objectstack/cli`, never of `@objectstack/core`. + // + // Registered under 18, not 17: v17.0.0 was cut before this landed, so the + // removal ships on the 17.x line (launch-window convention: accept-set + // narrowings ride minor releases) and the prescription lives at the major + // boundary where `migrate meta` users look — the same grading #12340 used one + // day earlier in this module. + // + // Tombstoned with `retiredKey()` in `HotReloadConfigSchema` (the surface + // baseline line carries `[RETIRED]`). Deliberately NO D2 conversion: the chain + // walks a normalized STACK, and `HotReloadConfig` is not an authorable surface + // — no metadata-type binding, stack collection or manifest embed ever carried + // it, and nothing in the tree parses `HotReloadConfigSchema` outside its own + // unit test — so a conversion would be a transform with no seam that ever + // runs. The D3 semantic entry `hot-reload-watch-placeholder-retired` is the + // declaration, and the registration-time refusal in + // `HotReloadManager.registerPlugin` is the door for the audience that exists. + 'kernel/HotReloadConfig:watchPatterns', // #10724 — ADR-0049 enforce-or-remove on the plugin manifest's `contributes` // block; one of NINE members tombstoned together. Census, registration major, // and the why-no-D2-conversion reasoning are recorded once in the sibling