From 2c7fb18b2729219a6b566be1ffe1c686041751c3 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 11:08:10 +0000 Subject: [PATCH] fix(runtime,metadata-protocol): the `data` discovery slot is computed too, closing the hardcoded kernel block (#4130) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #4089 made both discovery builders derive the `metadata` entry from the implementation in the slot. `data` was the one entry left that judged itself: `status: 'available'`, `handlerReady: true`, unconditionally, in both builders. That was true — by a convention in a different package. ObjectQL is the slot's only producer (`ctx.registerService('data', this.ql)`), and plugin-dev fills a core slot only when it is empty while always loading `ObjectQLPlugin` as a child, so plugin-dev's `data` stub — `find()` returns `[]`, `insert()` mints an id and stores nothing — never reaches the slot. Neither builder verified any of that. It is the same shape the `metadata` hardcodes had while they were still "roughly right", on the platform's most load-bearing capability, and it breaks the moment a second `data` producer appears or a dev config drops the ObjectQL child. Both builders now read the slot's `__serviceInfo`: - a real engine carries no marker ⇒ `available` + `handlerReady: true`, which is byte-identical to the hardcode it replaces (pinned on a real kernel boot, not just on mocks); - a self-declared stub ⇒ its own `status` / `message`, and `handlerReady: false`, which `readServiceSelfInfo` already defaults for `stub`. `handlerReady` is derived here rather than pinned `true` as it is for `metadata`, because each entry says what its own route does. `/meta` answers from the protocol — down to a last-resort default type list — whatever fills the metadata slot, so a degraded implementation there does not unmount it. `/data` has no such floor: `callData` needs the `protocol` service or an objectql-shaped one and throws 503 without them, and the only way a stub occupies the `data` slot is a stack where ObjectQL never registered, i.e. exactly the stack where `/data` cannot serve. Deliberately NOT done: re-deriving serveability from `protocol`/`objectql` inside the builder. Discovery resolves services UNSCOPED (it has no request context), so on a multi-kernel host that could report the required data capability as `handlerReady: false` while per-request scoped resolution serves it fine — a worse lie than the one being fixed. Nothing routes off this field either: the `data` domain resolves its engine directly, and `isServiceServeable` (#4058 step 2) gates the optional domains only. Tests: both directions per builder, the cross-builder agreement extended to `data` (same instance ⇒ same status / handlerReady / message / route), and the real-boot assertion that a live ObjectQL keeps reporting `available`. Closes #4130 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RfdfmzBq2hdp6gzHaK1S9p --- .changeset/discovery-data-slot-computed.md | 15 +++++ docs/adr/0076-objectql-core-tiering.md | 2 +- packages/metadata-protocol/src/protocol.ts | 19 +++++- .../objectql/src/plugin.integration.test.ts | 7 ++ .../objectql/src/protocol-discovery.test.ts | 36 ++++++++++ packages/runtime/src/http-dispatcher.test.ts | 65 +++++++++++++++++++ packages/runtime/src/http-dispatcher.ts | 38 ++++++++++- 7 files changed, 178 insertions(+), 4 deletions(-) create mode 100644 .changeset/discovery-data-slot-computed.md diff --git a/.changeset/discovery-data-slot-computed.md b/.changeset/discovery-data-slot-computed.md new file mode 100644 index 0000000000..e8dfab849f --- /dev/null +++ b/.changeset/discovery-data-slot-computed.md @@ -0,0 +1,15 @@ +--- +'@objectstack/metadata-protocol': patch +'@objectstack/runtime': patch +--- + +Both discovery builders now derive the `data` service entry from the implementation in the slot, closing the hardcoded "kernel-provided" block (#4130). + +#4089 computed `metadata`; `data` was the last entry that judged itself, reporting `status: 'available'` and `handlerReady: true` unconditionally. That was true — but by a convention in a different package, not by anything either builder checked: ObjectQL is the slot's only producer, and plugin-dev always loads `ObjectQLPlugin` as a child, so plugin-dev's `data` stub (`find()` returns `[]`, `insert()` mints an id and stores nothing) never reaches the slot. A second producer, or a trimmed dev config, and the hardcode starts lying about the platform's most load-bearing capability. + +Both builders now read the registered service's `__serviceInfo`: + +- a real engine carries no marker ⇒ `available` + `handlerReady: true`, byte-identical to the hardcode it replaces (verified on a real kernel boot); +- a self-declared stub ⇒ its own `status` and `message`, with `handlerReady: false` (the default for `stub`), so a consumer that gates on `handlerReady` stops treating an empty query engine as a real one. + +`handlerReady` is derived here rather than pinned `true` as it is for `metadata`, because the two routes differ: `/meta` answers from the protocol whatever fills the metadata slot, while `/data` needs the `protocol` or an objectql-shaped service and 503s without them — and the only stack where a stub occupies the `data` slot is one where ObjectQL never registered. No routing, gating or dispatch behavior changes: the `data` domain resolves its engine directly and never consulted this slot. diff --git a/docs/adr/0076-objectql-core-tiering.md b/docs/adr/0076-objectql-core-tiering.md index 44550bb19d..f7427c8213 100644 --- a/docs/adr/0076-objectql-core-tiering.md +++ b/docs/adr/0076-objectql-core-tiering.md @@ -134,7 +134,7 @@ Decision: each capability plugin registers its routes as a **normalized handler* **Inventory of current fakes / mis-reports:** - `plugin-dev` registers ~8 dev stubs — `storage` / `search` / `automation` / `graphql` / `analytics` / `realtime` / `notification` / `ai` (they already carry a `_dev: true` marker that nothing respects). *(Update #4000: the `analytics` one is **gone** — after the fallback's retirement (#3891/#3989) it was the last thing refilling that slot, and refilling it re-created the retired shape in dev: the dispatcher gated on service presence, so the stub was called like an engine and answered 200 with fabricated rows. The remaining stubs are unchanged, and the class-wide question they raise is tracked separately — see conclusion 3 below.)* *(Update #4058 step 1: the blanket `_dev: true` is **gone** too. Every dev implementation now carries the standard `__serviceInfo`, classified in one reviewable table (`DEV_STUB_SELF_INFO`) by what it actually is — `degraded` for the ones that really work with reduced capability (`file-storage` / `search` / `metadata` / `workflow` / `realtime`, plus the wrapped kernel fallbacks), `stub` for the ones whose answer is fabricated (`ai` / `automation` / `notification` / `data` / `auth` / `security.*`). The single marker made those two indistinguishable and declared the working ones fake, which is why "adopt the dispatcher gate everywhere?" could not be answered. The gates themselves are untouched — that is #4058 step 2.)* - `ObjectQLPlugin` registers a ~66-line `analytics` **fallback** (the D10 note — deliberate, but it reports as fully available). -- `http-dispatcher.ts` `svcAvailable` — the hardcode above. *(Update #4089: `svcAvailable` respects the marker, but two entries never went through it at all — a "kernel-provided (always available)" block above the loop, hardcoded per builder and **contradicting itself across them**: the dispatcher declared `metadata` permanently `degraded` with "In-memory registry; DB persistence pending", while metadata-protocol declared the same slot permanently `available`. So one host called a `sys_metadata`-backed registry degraded and the other called the in-memory fallback fully real — the marker #4058 had just added to `createMemoryMetadata` went unread on both sides, because neither side read anything. Both now compute the slot from the registered implementation's `__serviceInfo` and agree; `handlerReady: true` stays unconditional on both, since `/api/v1/meta` is served by the protocol whichever implementation occupies the slot. `data` keeps its hardcode: ObjectQL is the only producer that ever fills it, and `ObjectQLPlugin` — which plugin-dev always loads as a child — registers the real engine, so plugin-dev's `data` stub is unreachable in a stack that has a discovery builder at all.)* +- `http-dispatcher.ts` `svcAvailable` — the hardcode above. *(Update #4089: `svcAvailable` respects the marker, but two entries never went through it at all — a "kernel-provided (always available)" block above the loop, hardcoded per builder and **contradicting itself across them**: the dispatcher declared `metadata` permanently `degraded` with "In-memory registry; DB persistence pending", while metadata-protocol declared the same slot permanently `available`. So one host called a `sys_metadata`-backed registry degraded and the other called the in-memory fallback fully real — the marker #4058 had just added to `createMemoryMetadata` went unread on both sides, because neither side read anything. Both now compute the slot from the registered implementation's `__serviceInfo` and agree; `handlerReady: true` stays unconditional on both, since `/api/v1/meta` is served by the protocol whichever implementation occupies the slot. `data` kept its hardcode in that pass: ObjectQL is the only producer that ever fills it, and `ObjectQLPlugin` — which plugin-dev always loads as a child — registers the real engine, so plugin-dev's `data` stub is unreachable in a stack that has a discovery builder at all.)* *(Update #4130: `data` is computed now too, which closes the block. Its hardcoded `available` was true, but by a **load-order convention in another package** rather than by anything either builder verified — the same shape the `metadata` hardcodes had while they were still "roughly right". Both builders derive the entry from the slot's `__serviceInfo`: an unmarked engine reproduces the old `available` / `handlerReady: true` byte for byte, while plugin-dev's `data` stub — `status: 'stub'`, so `handlerReady` defaults to `false` — is reported as the non-handler it declares itself to be. Unlike `metadata`, `handlerReady` here is NOT pinned `true`: `/meta` answers from the protocol (down to a last-resort default type list) whatever fills its slot, whereas `callData` needs the `protocol` service or an objectql-shaped one and throws 503 without them — and the only way a stub reaches the `data` slot is a stack where ObjectQL never registered, i.e. exactly the stack where `/data` cannot serve. Deliberately NOT done: re-deriving serveability from `protocol`/`objectql` inside the builder. Discovery resolves services **unscoped**, so that could report the required data capability as `handlerReady: false` on a multi-kernel host while per-request scoped resolution serves it fine — a worse lie than the one being fixed. No domain gates on this slot either: `isServiceServeable` (#4058 step 2) covers the optional domains, and `/data` resolves its engine directly.)* - *(Added by #4058: the **kernel's own** fallbacks — `createMemoryCache` / `Queue` / `Job` / `I18n` / `Metadata`, auto-registered by ObjectKernel — were missing from this inventory and were the worst case in it: they carried `_fallback: true`, a marker **no** reader recognized (not even `readServiceSelfInfo`), so both discovery builders reported them as fully `available`. They now self-describe as `degraded` with `handlerReady: false` where no HTTP surface exists (`cache` / `queue` / `job`). The lone duck-typed consumer of `_fallback`/`_dev` — an i18n diagnostic in `app-plugin.ts` — reads `readServiceSelfInfo` instead.)* **Decision — honest capabilities:** diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 195025ca3b..02a94b183b 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -1481,6 +1481,16 @@ export class ObjectStackProtocolImplementation implements // mounted whichever implementation occupies the slot. const metadataSelf = readServiceSelfInfo(registeredServices.get('metadata')); + // [#4130] `data` was the last self-judging entry in this block. Same + // reasoning, one degree weaker: its hardcoded `available` is currently + // true, but only because ObjectQL is the slot's sole producer and + // plugin-dev (whose `data` stub declares `stub`) always loads + // ObjectQLPlugin as a child. That is a load-order convention in another + // package, not something this builder verifies — so verify it here. + // Unmarked implementation ⇒ `available` + `handlerReady: true`, i.e. + // exactly what the hardcode said, now derived rather than asserted. + const dataSelf = readServiceSelfInfo(registeredServices.get('data')); + const services: Record = { // --- Kernel-provided (objectql is an example kernel implementation) --- metadata: { @@ -1491,7 +1501,14 @@ export class ObjectStackProtocolImplementation implements provider: 'objectql', ...(metadataSelf?.message ? { message: metadataSelf.message } : {}), }, - data: { enabled: true, status: 'available' as const, route: '/api/v1/data', provider: 'objectql' }, + data: { + enabled: true, + status: dataSelf?.status ?? ('available' as const), + handlerReady: dataSelf?.handlerReady ?? true, + route: '/api/v1/data', + provider: 'objectql', + ...(dataSelf?.message ? { message: dataSelf.message } : {}), + }, }; // [#4000, #4058] The dispatcher answers a self-declared non-handler in diff --git a/packages/objectql/src/plugin.integration.test.ts b/packages/objectql/src/plugin.integration.test.ts index 4bbcb64909..202647e6ab 100644 --- a/packages/objectql/src/plugin.integration.test.ts +++ b/packages/objectql/src/plugin.integration.test.ts @@ -54,6 +54,13 @@ describe('ObjectQLPlugin - Metadata Service Integration', () => { // Serving is unaffected — `/api/v1/meta` is the protocol's route. expect(discovery.services.metadata.handlerReady).toBe(true); expect(discovery.routes.metadata).toBe('/api/v1/meta'); + + // …and on the same boot, `data` — derived from its slot too now (#4130) + // — is unchanged: ObjectQLPlugin registers the real engine there and it + // carries no marker, so the derivation reproduces the old hardcode. + expect(discovery.services.data.status).toBe('available'); + expect(discovery.services.data.handlerReady).toBe(true); + expect(discovery.services.data.message).toBeUndefined(); }); it('should serve in-memory metadata definitions', async () => { diff --git a/packages/objectql/src/protocol-discovery.test.ts b/packages/objectql/src/protocol-discovery.test.ts index 92433d88e5..695eb81db0 100644 --- a/packages/objectql/src/protocol-discovery.test.ts +++ b/packages/objectql/src/protocol-discovery.test.ts @@ -181,6 +181,42 @@ describe('ObjectStackProtocolImplementation - Dynamic Service Discovery', () => expect(discovery.services.metadata.handlerReady).toBe(true); }); + // #4130 — `data` was the last hardcoded entry in the kernel block. Its + // `available` happened to be true, but only because ObjectQL is the slot's + // sole producer and plugin-dev always loads it as a child, so its `data` stub + // never lands there. A convention in another package is not something this + // builder verifies — now it does. + it('should report a self-declared data stub as a stub, never available', async () => { + const mockServices = new Map(); + mockServices.set('data', { + __serviceInfo: { status: 'stub', message: 'Dev stub — find() always returns []. Register ObjectQLPlugin for a real engine.' }, + find: async () => [], + }); + + protocol = new ObjectStackProtocolImplementation(engine, () => mockServices); + const discovery = await protocol.getDiscovery(); + + expect(discovery.services.data.enabled).toBe(true); + expect(discovery.services.data.status).toBe('stub'); + expect(discovery.services.data.handlerReady).toBe(false); + expect(discovery.services.data.message).toContain('ObjectQLPlugin'); + }); + + it('should keep reporting a real (unmarked) data engine as available', async () => { + const mockServices = new Map(); + mockServices.set('data', { find: async () => [], insert: async () => ({ id: '1' }) }); + + protocol = new ObjectStackProtocolImplementation(engine, () => mockServices); + const discovery = await protocol.getDiscovery(); + + // Byte-for-byte what the hardcode said, now derived from the slot. + expect(discovery.services.data.status).toBe('available'); + expect(discovery.services.data.handlerReady).toBe(true); + expect(discovery.services.data.route).toBe('/api/v1/data'); + expect(discovery.services.data.provider).toBe('objectql'); + expect(discovery.services.data.message).toBeUndefined(); + }); + // #3891 — the degraded ObjectQL fallback is retired. Analytics is an // ordinary optional service now: absent ⇒ unavailable, and the route must // NOT be advertised (an advertised route with no handler 404s — the exact diff --git a/packages/runtime/src/http-dispatcher.test.ts b/packages/runtime/src/http-dispatcher.test.ts index 3d651e6055..48518416a1 100644 --- a/packages/runtime/src/http-dispatcher.test.ts +++ b/packages/runtime/src/http-dispatcher.test.ts @@ -2415,6 +2415,71 @@ describe('HttpDispatcher', () => { expect(fromDispatcher.message).toBe(fromProtocol.message); expect(fromDispatcher.route).toBe(fromProtocol.route); }); + + // ── The `data` slot: the same hardcode, one degree weaker (#4130) ───── + // + // `available` / `handlerReady: true` was true here only by a convention + // in another package (ObjectQL is the slot's only producer, and + // plugin-dev always loads it as a child so its `data` stub never lands). + // These pin the computation that replaces the convention. + + it('keeps reporting a real (unmarked) data engine as available — the hardcode it replaces', async () => { + (kernel as any).getService = vi.fn().mockImplementation((name: string) => + name === 'data' ? mockObjectQL : null, + ); + + const info = await dispatcher.getDiscoveryInfo('/api/v1'); + expect(info.services.data.enabled).toBe(true); + expect(info.services.data.status).toBe('available'); + expect(info.services.data.handlerReady).toBe(true); + expect(info.services.data.route).toBe('/api/v1/data'); + expect(info.services.data.provider).toBe('kernel'); + expect(info.services.data.message).toBeUndefined(); + }); + + it('reports plugin-dev\'s data stub as a stub with no ready handler, not as a query engine', async () => { + // The real marker plugin-dev attaches (DEV_STUB_SELF_INFO.data): + // `stub` with no explicit handlerReady, which readServiceSelfInfo + // defaults to false. + const devDataStub = { + __serviceInfo: { + status: 'stub', + message: 'Dev stub — find() always returns [], insert() mints an id and stores nothing. Register ObjectQLPlugin for a real engine.', + }, + find: vi.fn().mockResolvedValue([]), + insert: vi.fn().mockResolvedValue({ id: 'x' }), + }; + (kernel as any).getService = vi.fn().mockImplementation((name: string) => + name === 'data' ? devDataStub : null, + ); + + const info = await dispatcher.getDiscoveryInfo('/api/v1'); + expect(info.services.data.status).toBe('stub'); + expect(info.services.data.handlerReady).toBe(false); + expect(info.services.data.message).toContain('ObjectQLPlugin'); + }); + + it('answers the data slot identically to the metadata-protocol builder', async () => { + const { ObjectStackProtocolImplementation } = await import('@objectstack/metadata-protocol'); + const degradedData = { + __serviceInfo: { status: 'degraded', message: 'read-only engine' }, + find: vi.fn().mockResolvedValue([]), + }; + (kernel as any).getService = vi.fn().mockImplementation((name: string) => + name === 'data' ? degradedData : null, + ); + + const fromDispatcher = (await dispatcher.getDiscoveryInfo('/api/v1')).services.data; + const fromProtocol = (await new ObjectStackProtocolImplementation( + mockObjectQL as any, + () => new Map([['data', degradedData]]), + ).getDiscovery()).services.data; + + expect(fromDispatcher.status).toBe(fromProtocol.status); + expect(fromDispatcher.handlerReady).toBe(fromProtocol.handlerReady); + expect(fromDispatcher.message).toBe(fromProtocol.message); + expect(fromDispatcher.route).toBe(fromProtocol.route); + }); }); // ═══════════════════════════════════════════════════════════════ diff --git a/packages/runtime/src/http-dispatcher.ts b/packages/runtime/src/http-dispatcher.ts index 5c554e0a50..894c818c1a 100644 --- a/packages/runtime/src/http-dispatcher.ts +++ b/packages/runtime/src/http-dispatcher.ts @@ -844,7 +844,7 @@ export class HttpDispatcher { authSvc, searchSvc, realtimeSvc, filesSvc, analyticsSvc, workflowSvc, aiSvc, notificationSvc, i18nSvc, uiSvc, automationSvc, cacheSvc, queueSvc, jobSvc, mcpSvc, - metadataSvc, + metadataSvc, dataSvc, ] = await Promise.all([ this.resolveService(CoreServiceName.enum.auth), this.resolveService(CoreServiceName.enum.search), @@ -866,6 +866,8 @@ export class HttpDispatcher { // [#4089] Resolved for its self-description alone: the `metadata` // slot is reported from what actually fills it, not hardcoded. this.resolveService(CoreServiceName.enum.metadata), + // [#4130] Same, for the last hardcoded entry in the kernel block. + this.resolveService(CoreServiceName.enum.data), ]); const hasAuth = !!authSvc; @@ -1052,7 +1054,39 @@ export class HttpDispatcher { provider: 'kernel', ...(metadataSelf?.message ? { message: metadataSelf.message } : {}), }, - data: svcAvailable(routes.data, 'kernel'), + // [#4130] The last entry in this block that judged itself. It + // read `available` / `handlerReady: true` unconditionally, and + // that was true only by a convention outside this file: + // ObjectQL is the sole producer of the `data` slot, and + // plugin-dev — whose `data` stub self-declares `stub` (find() + // returns [], insert() stores nothing) — always loads + // ObjectQLPlugin as a child, so the stub never reaches the slot. + // A convention is not a computation: a second producer, or a + // trimmed dev config, and the hardcode starts lying. Passing the + // resolved service lets `svcAvailable` derive both fields, and + // for a real engine (which carries no marker) the result is + // byte-identical to the hardcode it replaces. + // + // Why `handlerReady` is derived here but pinned `true` for + // `metadata` above: each says what its route actually does. + // `/meta` answers from the protocol (and a last-resort default + // type list) whatever fills the metadata slot, so a degraded + // implementation there does not unmount it. `/data` has no such + // floor — `callData` needs the `protocol` service or an + // objectql-shaped one and throws 503 without them — and the only + // way a stub occupies this slot is a stack where ObjectQL never + // registered, i.e. exactly the stack where `/data` cannot serve. + // + // Deliberate limit: this reports the slot, it does not re-derive + // serveability from `protocol`/`objectql`. Mirroring that + // predicate here would read them UNSCOPED (discovery has no + // environment context), which on a multi-kernel host risks + // flipping the required data capability to `handlerReady: false` + // while per-request scoped resolution serves it fine — a worse + // lie than the one being fixed. The `data` domain does not gate + // on this slot either (`isServiceServeable` covers the optional + // domains, #4058 step 2), so nothing routes off this field. + data: svcAvailable(routes.data, 'kernel', dataSvc), // Plugin-provided — only available when a plugin registers the service auth: hasAuth ? svcAvailable(routes.auth, undefined, authSvc) : svcUnavailable('auth'), // [#4000, #4058] Presence-gated, not serveability-gated: a