Skip to content

Commit 43fc039

Browse files
os-zhuangclaude
andauthored
fix(runtime): /ui discovery gates on the protocol service that serves it, not the vestigial ui slot (#4093) (#4146)
domains/ui.ts serves GET /ui/view/:object off the `protocol` service and 503s without it; the `ui` core-service slot never enters that decision, and nothing in the platform registers a `ui` service — plugin-dev's shapeless placeholder was its only occupant ever, and ADR-0115 (#4137) retired it. Gating routes.ui on slot presence was therefore wrong in both directions: a dev boot with the placeholder but no protocol advertised a route that could only 503, and every boot without a placeholder — production always, and all dev boots post- ADR-0115 — hid a route that serves fine. routes.ui and services.ui now gate on `typeof protocol?.getUiView === 'function'` — the domain handler's own guard, byte for byte, the same rule the mcp advertisement follows (same predicate ⇒ same answer). services.ui reports the serving implementation (provider metadata-protocol, honoring any __serviceInfo it declares), and the unavailable message names the actual remedy — register MetadataPlugin (@objectstack/metadata-protocol) — instead of svcUnavailable's "install a ui plugin", which names a plugin that does not exist. This PR originally also carried plugin-dev's incremental data/ui stub retirements; ADR-0115 (#4137) retired the whole stub table while the branch was open, so that half resolved to main's state and the ADR-0076 annotation now closes out pointing at ADR-0115. The changeset is a runtime-only patch. The one surviving fabricator is outside plugin-dev: the dispatcher's /auth mock fallback, #4113. Tests: with a protocol, /ui is advertised and really serves (getUiView called with the parsed object/type); without one it is not advertised and the remedy is named; a ui-slot occupant with no protocol — the old dev-boot shape — stays un-advertised AND 503s (the slot buys no route); a wrong-shaped protocol mirrors the domain. runtime 936 / plugin-dev 10 / metadata-protocol 110 pass; build 71/71; eslint clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent b4be309 commit 43fc039

4 files changed

Lines changed: 113 additions & 5 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@objectstack/runtime': patch
3+
---
4+
5+
Discovery's `/ui` advertisement reads what `/ui` reads: the `protocol` service, not the vestigial `ui` slot (#4093).
6+
7+
`domains/ui.ts` serves `GET /ui/view/:object` off the `protocol` service and 503s without it; the `ui` core-service slot never enters that decision, and nothing in the platform registers a `ui` service — plugin-dev's shapeless placeholder was its only occupant ever, and ADR-0115 retired it. Gating `routes.ui` on slot presence was therefore wrong in both directions: a dev boot with the placeholder but no protocol advertised a route that could only 503, and every boot without a placeholder — production always, and all dev boots post-ADR-0115 — hid a route that serves fine.
8+
9+
`routes.ui` and `services.ui` now gate on `typeof protocol?.getUiView === 'function'` — the domain handler's own guard, byte for byte, the same rule the `mcp` advertisement follows. `services.ui` reports the serving implementation (provider `metadata-protocol`, honoring any `__serviceInfo` it declares), and the unavailable message names the actual remedy — register MetadataPlugin (`@objectstack/metadata-protocol`) — instead of "install a ui plugin", which names a plugin that does not exist.
10+
11+
FROM → TO: `routes.ui` / `services.ui` may newly appear in deployments where the protocol service is registered (the route always served there; discovery just never said so) and newly disappear in protocol-less boots (it never worked there). No handler behavior changes.

docs/adr/0076-objectql-core-tiering.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ Decision: each capability plugin registers its routes as a **normalized handler*
132132
**Root cause of agents being misled.** Several plugins register stub / dev / fallback services under canonical names, and the discovery builder reports *any* present service as fully real: `runtime/http-dispatcher.ts`'s `svcAvailable` hardcodes `{ enabled: true, status: 'available', handlerReady: true }` for every registered service — it **ignores stub markers** (its own comment even says "handlerReady:false … may be served by a stub", but the code never computes it). So `discovery.services.*` claims capabilities that are only stubbed, and consumers (AI agents, the console) trust them. A dev AI stub advertised this way has already confused an agent.
133133

134134
**Inventory of current fakes / mis-reports:**
135-
- `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.)* *(Update #4093: three of those `stub` entries are **gone**, not relabelled — `security.permissions` / `security.rls` / `security.fieldMasker`. Classifying them honestly was necessary but not sufficient, for the same reason honest labelling was not enough for the analytics shim (#3891): the label was accurate while the behaviour still inverted the decision it stood in for — `checkObjectPermission()` returned `true` for everything, `compileFilter()` returned `null` so no row predicate applied, `maskResults()` returned rows unmasked. That is this decision's own line — a fallback may degrade features, never security semantics — and `spec/src/contracts/security-service.ts` states it from the other side: these three are plugin-security's internals, and access-narrowing answers must fail CLOSED. `plugin-dev` loads SecurityPlugin through the same optional dynamic import as everything else, so the package merely being absent swapped real RBAC/RLS/masking for allow-all behind one `warn`. The slots now stay empty (what production has without SecurityPlugin), the boot log states that nothing is enforcing them, and `plugin-dev` additionally refuses to initialize at all under `NODE_ENV=production` (`OS_ALLOW_DEV_PLUGIN=1` to override) — it is a published package that had no environment check of its own. Whether the remaining fabricating stubs (`data`, `auth`, `ui`) should occupy slots at all is still #4093; `auth` in particular is blocked on #4113, since the dispatcher's own `/auth` mock fallback — not the dev stub — is what actually answers `/auth/*` when the slot is empty.)*
135+
- `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.)* *(Update #4093: three of those `stub` entries are **gone**, not relabelled — `security.permissions` / `security.rls` / `security.fieldMasker`. Classifying them honestly was necessary but not sufficient, for the same reason honest labelling was not enough for the analytics shim (#3891): the label was accurate while the behaviour still inverted the decision it stood in for — `checkObjectPermission()` returned `true` for everything, `compileFilter()` returned `null` so no row predicate applied, `maskResults()` returned rows unmasked. That is this decision's own line — a fallback may degrade features, never security semantics — and `spec/src/contracts/security-service.ts` states it from the other side: these three are plugin-security's internals, and access-narrowing answers must fail CLOSED. `plugin-dev` loads SecurityPlugin through the same optional dynamic import as everything else, so the package merely being absent swapped real RBAC/RLS/masking for allow-all behind one `warn`. The slots now stay empty (what production has without SecurityPlugin), the boot log states that nothing is enforcing them, and `plugin-dev` additionally refuses to initialize at all under `NODE_ENV=production` (`OS_ALLOW_DEV_PLUGIN=1` to override) — it is a published package that had no environment check of its own. Whether the remaining fabricating stubs (`data`, `auth`, `ui`) should occupy slots at all is still #4093; `auth` in particular is blocked on #4113, since the dispatcher's own `/auth` mock fallback — not the dev stub — is what actually answers `/auth/*` when the slot is empty.)* *(Update — closed by [ADR-0115](./0115-plugin-dev-assembly-not-stub-table.md) (#4137): the question "should the remaining fabricating stubs occupy slots at all" was answered wholesale, not per slot — the stub table is retired as a design, plugin-dev registers no implementations of its own, and every slot it filled is empty in dev exactly as in production. The one fabricator left standing is outside plugin-dev: the dispatcher's own `/auth` mock fallback, #4113. One discovery seam followed in #4146: `routes.ui`/`services.ui` now gate on the `protocol` service's own `getUiView` guard — what `domains/ui.ts` actually reads, the `hasMcp` rule — rather than on the vestigial `ui` slot, which nothing registers in any mode; slot-presence gating had advertised `/ui` for the placeholder in boots where it could only 503, and hidden it in every boot where the protocol really serves it.)*
136136
- `ObjectQLPlugin` registers a ~66-line `analytics` **fallback** (the D10 note — deliberate, but it reports as fully available).
137137
- `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.)*
138138
- *(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.)*

packages/runtime/src/http-dispatcher.test.ts

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2715,6 +2715,80 @@ describe('HttpDispatcher', () => {
27152715
});
27162716
});
27172717

2718+
// ═══════════════════════════════════════════════════════════════
2719+
// [#4093] /ui advertisement reads what /ui reads
2720+
//
2721+
// domains/ui.ts serves GET /ui/view/:object off the `protocol` service and
2722+
// 503s without it; the `ui` SLOT never enters that decision — nothing in
2723+
// the platform registers a `ui` service (plugin-dev's shapeless
2724+
// placeholder, retired in #4093, was its only occupant ever). Discovery
2725+
// used to gate `routes.ui` on that slot, which was wrong in both
2726+
// directions: a boot with the placeholder but no protocol advertised a
2727+
// route that could only 503, and a production boot with a working
2728+
// protocol hid a route that serves. Same predicate ⇒ same answer
2729+
// (the `hasMcp` rule).
2730+
// ═══════════════════════════════════════════════════════════════
2731+
2732+
describe('/ui discovery gates on the protocol service, not the vestigial ui slot (#4093)', () => {
2733+
const serveMap = (services: Record<string, unknown>) => {
2734+
(kernel as any).getService = vi.fn().mockImplementation((n: string) => services[n] ?? null);
2735+
(kernel as any).services = new Map(Object.entries(services));
2736+
};
2737+
2738+
it('advertises /ui and serves it when a protocol with getUiView is registered', async () => {
2739+
const protocol = { getUiView: vi.fn().mockResolvedValue({ object: 'account', type: 'list', view: {} }) };
2740+
serveMap({ protocol });
2741+
2742+
const info = await dispatcher.getDiscoveryInfo('/api/v1');
2743+
expect(info.routes.ui).toBe('/api/v1/ui');
2744+
expect(info.services.ui.enabled).toBe(true);
2745+
expect(info.services.ui.status).toBe('available');
2746+
expect(info.services.ui.handlerReady).toBe(true);
2747+
expect(info.services.ui.provider).toBe('metadata-protocol');
2748+
2749+
// The advertised route really answers — same predicate, same fact.
2750+
const served = await dispatcher.handleUi('/view/account', {}, { request: {} });
2751+
expect(served.handled).toBe(true);
2752+
expect(served.response?.status).toBe(200);
2753+
expect(protocol.getUiView).toHaveBeenCalledWith({ object: 'account', type: 'list' });
2754+
});
2755+
2756+
it('does not advertise /ui without a protocol, and names the actual remedy', async () => {
2757+
serveMap({});
2758+
2759+
const info = await dispatcher.getDiscoveryInfo('/api/v1');
2760+
expect(info.routes.ui).toBeUndefined();
2761+
expect(info.services.ui.enabled).toBe(false);
2762+
expect(info.services.ui.handlerReady).toBe(false);
2763+
// Not svcUnavailable's "Install a ui plugin" — no such plugin exists.
2764+
expect(info.services.ui.message).toContain('MetadataPlugin');
2765+
});
2766+
2767+
it('a ui-slot occupant buys no route: the old dev-boot shape stays un-advertised and un-served', async () => {
2768+
// What plugin-dev used to register: a shapeless placeholder in the
2769+
// `ui` slot, no protocol anywhere. /ui could only 503 — discovery
2770+
// must say so instead of advertising it.
2771+
const placeholder = { _serviceName: 'ui', __serviceInfo: { status: 'stub', handlerReady: false, message: 'Dev placeholder' } };
2772+
serveMap({ ui: placeholder });
2773+
2774+
const info = await dispatcher.getDiscoveryInfo('/api/v1');
2775+
expect(info.routes.ui).toBeUndefined();
2776+
expect(info.services.ui.enabled).toBe(false);
2777+
2778+
const served = await dispatcher.handleUi('/view/account', {}, { request: {} });
2779+
expect(served.handled).toBe(true);
2780+
expect(served.response?.status).toBe(503);
2781+
});
2782+
2783+
it('a wrong-shaped protocol (no getUiView) is not advertised — mirrors the domain 503', async () => {
2784+
serveMap({ protocol: { saveMetaItem: vi.fn() } });
2785+
2786+
const info = await dispatcher.getDiscoveryInfo('/api/v1');
2787+
expect(info.routes.ui).toBeUndefined();
2788+
expect(info.services.ui.enabled).toBe(false);
2789+
});
2790+
});
2791+
27182792
// ═══════════════════════════════════════════════════════════════
27192793
// i18n across server/dev/mock environments
27202794
// ═══════════════════════════════════════════════════════════════

0 commit comments

Comments
 (0)