fix(spec,core): startWatching refuses instead of reporting success; watchPatterns retired (#12571)

`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
the sibling defect in this file announced its inert fallback 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. The zero was pinned with a
firing positive control before anything was removed: `reloadTimers.set`
resolves a real writer in the same file and the same scan, while
`watchHandles.set` resolves nothing anywhere in the tree.

`watchPatterns` therefore had no reader that acted on it — its only two uses
were log lines — so ADR-0049 enforce-or-remove applies. Neither of the other
two states was available: 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.

The key is tombstoned rather than deleted, and the build chose 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 (ADR-0104) — the very defect being retired, one layer down.
`kernel/HotReloadConfig:watchPatterns` is registered in
`RETIRED_KEYS_BY_MAJOR[18]` with the D3 semantic entry
`hot-reload-watch-placeholder-retired`. No D2 conversion: `HotReloadConfig` is
not an authorable surface, so a conversion would be a transform with no seam
that ever runs.

Because nothing in the tree parses `HotReloadConfigSchema` outside its own unit
test, the tombstone alone reaches nobody, so the runtime doors carry the
prescription: `startWatching` throws an ADR-0112 envelope pointing at
`scheduleReload`, and `registerPlugin` refuses a leftover `watchPatterns` the
same way, before the `enabled` check. `startWatching` is kept as a throwing
door rather than deleted so that caller meets a prescription instead of a bare
TypeError.

Runtime reload behaviour is unchanged: nothing was ever watched, so nothing
that used to happen stops happening. `stopWatching` keeps the half that always
did something, and `shutdown` lost only a loop that iterated `watchHandles` and
therefore ran zero times.


Claude-Session: https://claude.ai/code/session_01W6HFzyH98W1YaQXhJUJt6o

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-warren

.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)
+
+<!-- adr-0087: registered hot-reload-watch-placeholder-retired -->
+
+**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.

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 |

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',

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<string, unknown> = {}): 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();
+    }
+  });
+});