diff --git a/.changeset/runtime-typed-protocol-handles-ui-meta-mcp.md b/.changeset/runtime-typed-protocol-handles-ui-meta-mcp.md new file mode 100644 index 0000000000..fe370d612f --- /dev/null +++ b/.changeset/runtime-typed-protocol-handles-ui-meta-mcp.md @@ -0,0 +1,16 @@ +--- +"@objectstack/runtime": patch +--- + +The `/ui`, `/meta` and `/mcp` dispatcher domains now reach the `protocol` service through a TYPED handle, so their request literals are compiled against the contracts `packages/spec` already declares. Behaviour is unchanged on every route; what changes is that a misspelt key or a misspelt verb is now a compile error instead of a silent no-op. + +`deps.resolveService(context, 'protocol')` answers `any` — `protocol` is deliberately left unmapped in `ServiceSlotContracts`, because a filled slot is NOT a promise that the slot holds a complete `MetadataProtocol`. That `any` is honest about the slot, but it also handed every request literal downstream an unchecked call target: `/ui` sent `getUiView({ object, type })` compiled against nothing at all, even though `getUiView` and `GetUiViewRequest` are both declared; `/meta` reached nine seams the same way, four of them through an explicit `(protocol as any)` cast; and `/mcp` was half-repaired, declaring `McpMergedMetadataRead` for its merged-read seam while the handle feeding it stayed annotated `any`. + +The repair is the consumer-side narrowing already proven in `domains/packages.ts`: one handle type per domain, `Pick`ed from the declared contract, resolved through a single one-line helper. + +- **Every member stays OPTIONAL, and every runtime capability probe survives.** A host may occupy this slot with a partial object — that is why the `typeof protocol. === 'function'` probes exist, and each one still asks its own question. The type answers "is this key declared?"; the probe answers "did THIS host bring the verb?". Tightening the members to required would pull the probes' premise out from under them, so the handle types are `Partial<...>` even where the declaration upstream is already optional. +- **Not a `packages/spec` change.** Mapping `'protocol'` in `ServiceSlotContracts` would assert that a filled slot IS a `MetadataProtocol`, whose members are mostly required, and it would have to answer for the verbs no contract declares at all. Nothing in `packages/spec` is touched. +- **The ledger ends honestly.** `listDrafts`, `migrateStoredMetadata` and `getProjectId` have no declared request shape anywhere — `@objectstack/metadata-protocol` types them inline on its implementation class and exports nothing for them — so their request keeps `any` and the gap stays greppable. What the entries still buy is the verb NAME. +- **One guard spelled out.** The `/meta` object read's scoped branch asked `typeof protocol.getMetaItem === 'function'` with no `protocol &&`, while its `!scoped` twin three lines below has always carried one. Behaviour-identical — `scoped` can only be true when the handle is there — and the `any` cast is what let the two siblings drift apart in spelling. + +No published type changes: `dist/index.d.ts` and `dist/index.d.cts` are byte-identical before and after, since the dispatcher domains are not re-exported from the package index. diff --git a/packages/runtime/src/domains/domain-protocol-handle-typing.test.ts b/packages/runtime/src/domains/domain-protocol-handle-typing.test.ts new file mode 100644 index 0000000000..e4143c798b --- /dev/null +++ b/packages/runtime/src/domains/domain-protocol-handle-typing.test.ts @@ -0,0 +1,263 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #15238 — the `/ui`, `/meta` and `/mcp` domains reach the `protocol` service + * through TYPED handles, and the runtime capability probes survive that typing. + * + * The template is `packages-protocol-handle-typing.test.ts` beside this file + * (#13598), and this is that instrument applied to the three domains the + * finding named. Two halves, because the card has two halves that pull in + * opposite directions and either one alone is a regression: + * + * 1. **Compile-time** (section 1). An undeclared key — or a misspelt verb — in + * one of these domains' request literals must be a COMPILE ERROR. That is + * the #11006 series' end state, and it stopped three seams short here. + * 2. **Runtime** (section 2). ⛔ A host may occupy the `protocol` slot with a + * PARTIAL object. Tightening the types and then deleting a + * `typeof … === 'function'` probe would trade the compile-time improvement + * for a runtime crash, so section 2 drives real dispatcher routes whose + * protocol brings none of the verbs and pins the documented answers. + * + * ## The defect, measured on the base tree with the card's own instrument + * + * `deps.resolveService(context, 'protocol')` answers `any` — `protocol` is + * deliberately unmapped in `ServiceSlotContracts`. Measured at `1008be3b`, + * per non-test file under `packages/runtime/src`: + * + * grep -c "resolveService([^,]*, 'protocol'" | grep -c "protocol as any\|protocol: any" + * domains/meta.ts 9 sites 4 casts + * domains/mcp.ts 1 site 1 cast + * domains/ui.ts 1 site 0 casts + * domains/packages.ts 1 site 0 casts <- the CONTROL, fixed by #13598 + * + * The `packages.ts` row is the control: the same instrument on the file that + * was already repaired, whose single remaining site is the one inside its + * narrowing helper. Without it the other three counts say nothing about + * whether the shape is repairable. + * + * Section 1 makes the repair durable. Each `@ts-expect-error` below is itself + * checked: if a seam goes back to `any` the directive stops matching an error + * and tsc reports TS2578 (unused directive) — so this file cannot rot into a + * green no-op the way an assertion-only pin could. + * + * ⚠️ These directives are NOT phantom checks: `packages/runtime`'s BUILD + * tsconfig excludes every `.test.ts` under `src`, but the sibling + * `tsconfig.test.json` compiles this layer and `package.json`'s `typecheck` + * script names it via `check:test-typecheck`. This file carries no entry in + * `test-typecheck-debt.json`, so any error it gains beyond the expected ones is + * red on arrival. + * + * ## Reverse verification — direction predicted BEFORE running + * + * Reverting the three domain files to the base tree makes section 1 red as + * TS2578 x12 (every directive becomes unused, because an `any` handle accepts + * everything) — the reversal shape, not a plain "assertion failed", which is + * why the directives are the pin and not `expectTypeOf` assertions. Section 2 + * is GREEN IN BOTH DIRECTIONS by construction: the probes it exercises are + * unchanged by this card, so it is the control that says the answers below + * were never bought with a behaviour change. + */ +import { describe, expect, it } from 'vitest'; +import { HttpDispatcher } from '../http-dispatcher.js'; +import type { UiDomainProtocol } from './ui.js'; +import type { MetaDomainProtocol } from './meta.js'; +import type { McpMergedMetadataRead } from './mcp.js'; + +// --------------------------------------------------------------------------- +// Section 1 — compile-time pins (never executed; the checker is the assertion) +// --------------------------------------------------------------------------- + +/** + * The literals these domains actually send, spelled exactly as the handlers + * spell them. A positive control for the `@ts-expect-error`s below: if this + * body ever stopped compiling, those directives could be "satisfied" by a type + * that rejects everything, which pins nothing. + */ +function declaredKeysCompile( + ui: UiDomainProtocol, + meta: MetaDomainProtocol, + mcp: McpMergedMetadataRead, +) { + return [ + // `/ui/view/:object[/:type]` — the one call in the whole domain. + ui.getUiView?.({ object: 'account', type: 'list' }), + ui.getUiView?.({ object: 'account', type: 'form' }), + // `/meta` reads. + meta.getMetaTypes?.({}), + meta.getMetaItems?.({ type: 'app', packageId: 'crm', organizationId: 'org_1', previewDrafts: true }), + meta.getMetaItem?.({ type: 'object', name: 'account', organizationId: 'org_1' }), + meta.getMetaItem?.({ type: 'app', name: 'crm', packageId: 'crm', organizationId: undefined, previewDrafts: false }), + meta.getMetaItemLayered?.({ type: 'view', name: 'account_list', organizationId: 'org_1' }), + // `/meta` write — `writeFace` is a DECLARED closed set and + // `'meta-dispatch'` is this door's member of it. + meta.saveMetaItem?.({ + type: 'app', + name: 'crm_console', + item: { _unpublished: false }, + organizationId: 'org_1', + writeFace: 'meta-dispatch', + packageId: 'crm', + }), + // The two undeclared-request verbs: the NAME is bought, the request + // shape is honestly still `any` (nothing declares one). + meta.listDrafts?.({ packageId: 'crm', type: 'view', organizationId: 'org_1' }), + meta.migrateStoredMetadata?.({ apply: false, types: ['view'], actor: 'u_1 (test)' }), + meta.getProjectId?.(), + // The `/mcp` merged skill read. + mcp.getMetaItems?.({ type: 'skill' }), + ]; +} + +/** + * ⛔ THE PIN. Each directive must match a real diagnostic; an unused one is + * TS2578 and fails `check:test-typecheck`. + */ +function undeclaredKeysAreCompileErrors( + ui: UiDomainProtocol, + meta: MetaDomainProtocol, + mcp: McpMergedMetadataRead, +) { + return [ + // ── /ui ────────────────────────────────────────────────────────── + ui.getUiView?.({ + // @ts-expect-error [#15238] `objectName` is not a member of the + // declared `GetUiViewRequest`; the key is `object`. Through the + // pre-change `any` handle this compiled and served nothing. + objectName: 'account', + type: 'list', + }), + // @ts-expect-error [#15238] `type` is a DECLARED closed set + // (`'list' | 'form'`) — `'grid'` is not in it. + ui.getUiView?.({ object: 'account', type: 'grid' }), + // @ts-expect-error [#15238] a misspelt VERB, which is what the untyped + // handle could never catch: any property access on `any` is a property + // access on `any`. + ui.getUiVeiw?.({ object: 'account', type: 'list' }), + // ⛔ Every member is OPTIONAL and STAYS optional: a filled slot is not a + // promise that the verb is there. This directive is what would go + // unused if someone "simplified" the handle to a non-partial + // `MetadataProtocol` — exactly the change that deletes the reason the + // runtime probes in section 2 exist. + // @ts-expect-error [#15238] possibly `undefined` — call it behind the probe. + ui.getUiView({ object: 'account', type: 'list' }), + + // ── /meta ──────────────────────────────────────────────────────── + meta.getMetaItem?.({ + type: 'object', + name: 'account', + // @ts-expect-error [#15238] `packagId` is a misspelling of the + // declared `packageId`. Through the pre-change `any` handle this + // compiled and the read silently ran unscoped. + packagId: 'crm', + }), + meta.getMetaItems?.({ + type: 'app', + // @ts-expect-error [#15238] not a member of `GetMetaItemsRequest` — + // the list read has no `packageIds` plural. + packageIds: ['crm'], + }), + meta.saveMetaItem?.({ + type: 'app', + name: 'crm_console', + item: {}, + // @ts-expect-error [#15238] `writeFace` is a DECLARED closed set; + // this door's member is `'meta-dispatch'` exactly. + writeFace: 'meta-dispatchh', + }), + // @ts-expect-error [#15238] a misspelt VERB on the undeclared-request + // half of the ledger: the request shape is still `any`, but the NAME is + // now checked — which is the whole win for these three verbs. + meta.migrateStoredMetadta?.({ apply: true }), + // @ts-expect-error [#15238] `getMetaItemLayered` was reached through a + // `(protocol as any)` cast before this card; the cast is gone and the + // declared request is enforced — `nmae` is not `name`. + meta.getMetaItemLayered?.({ type: 'view', nmae: 'account_list' }), + // @ts-expect-error [#15238] possibly `undefined` — the `/meta` probes + // stay, so every member stays optional here too. + meta.getMetaTypes({}), + + // ── /mcp ───────────────────────────────────────────────────────── + mcp.getMetaItems?.({ + // @ts-expect-error [#15238] `getMetaItems` takes `type`, not + // `types`. The second handle in this file was annotated `any` while + // its own `McpMergedMetadataRead` sat one seam away, so this key + // compiled at the resolve site. + types: ['skill'], + }), + // @ts-expect-error [#15238] the merged read is `Pick`ed to ONE verb — + // `getMetaItem` (singular) is deliberately not on this handle. + mcp.getMetaItem?.({ type: 'skill', name: 'a' }), + ]; +} + +// --------------------------------------------------------------------------- +// Section 2 — runtime control: the capability probes SURVIVE the typing +// --------------------------------------------------------------------------- + +/** `/meta/_drafts` and `/meta/_migrate-stored` gate BEFORE resolving the protocol. */ +const META_ADMIN = () => ({ + request: {}, + executionContext: { + userId: 'u_meta_admin', + systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'], + }, +}) as any; + +/** + * A host that OCCUPIES the `protocol` slot with an object carrying none of the + * verbs — the documented reason every call site probes rather than calls. Not + * an empty slot: an empty slot would take the `!protocol` arm of each guard and + * prove nothing about the `typeof … === 'function'` half. + */ +function partialProtocolDoor() { + const kernel: any = { + getService: (name: string) => { + if (name === 'protocol') return Promise.resolve({ someUnrelatedVerb: () => undefined }); + if (name === 'objectql') { + return Promise.resolve({ + registry: { getAllPackages: () => [], getPackage: () => undefined, getObject: () => undefined }, + }); + } + return null; + }, + context: { getService: () => null }, + }; + return new HttpDispatcher(kernel); +} + +describe('#15238 · 1 · the compile-time pins are type-level only', () => { + it('neither pin function is invoked — tsc is the assertion', () => { + expect(typeof declaredKeysCompile).toBe('function'); + expect(typeof undeclaredKeysAreCompileErrors).toBe('function'); + }); +}); + +describe('#15238 · 2 · a PARTIAL protocol host is still answered, never crashed', () => { + it('/ui/view answers 501 from the capability probe, not a crash', async () => { + const result = await partialProtocolDoor().handleUi('/view/account/list', {}, META_ADMIN()); + expect(result.handled).toBe(true); + expect(result.response?.status).toBe(501); + }); + + it('/meta/_drafts answers the documented 501 from its probe', async () => { + const result = await partialProtocolDoor().handleMetadata( + '/_drafts', META_ADMIN(), 'GET', undefined, {}, + ); + expect(result.response?.status).toBe(501); + expect(JSON.stringify(result.response?.body)).toContain('Draft listing not supported'); + }); + + it('/meta/_migrate-stored answers the documented 501 from its probe', async () => { + const result = await partialProtocolDoor().handleMetadata( + '/_migrate-stored', META_ADMIN(), 'POST', { apply: false }, {}, + ); + expect(result.response?.status).toBe(501); + expect(JSON.stringify(result.response?.body)).toContain('Stored-metadata migration not supported'); + }); + + it('/meta type listing falls through the probe to its own default', async () => { + const result = await partialProtocolDoor().handleMetadata('', META_ADMIN(), 'GET', undefined, {}); + expect(result.handled).toBe(true); + expect(result.response?.status).not.toBe(500); + }); +}); diff --git a/packages/runtime/src/domains/mcp.ts b/packages/runtime/src/domains/mcp.ts index d8736da01c..db15aef36e 100644 --- a/packages/runtime/src/domains/mcp.ts +++ b/packages/runtime/src/domains/mcp.ts @@ -331,7 +331,11 @@ function toMcpWebRequest(_deps: DomainHandlerDeps, raw: any, parsedBody: any): R * contract cannot drift from it, where a second hand-written `getMetaItems(…)` * signature silently could. */ -type McpMergedMetadataRead = Pick; +// [#15238] Exported so the handle-typing pin beside this file can name it — +// the same reason `domains/packages.ts` exports `PackagesDomainProtocol`. Not a +// published-surface change: `packages/runtime`'s index does not re-export +// `domains/`. +export type McpMergedMetadataRead = Pick; /** * [#8726] Read this environment's `skill` rows through the merged listing. @@ -611,7 +615,20 @@ export function buildMcpBridge(deps: DomainHandlerDeps, context: HttpProtocolCon // Resolved per request on the SAME per-environment seam `getMeta` // uses — never captured once at boot, which would serve one // environment's overlay rows to every other one. - const protocol: any = await deps.resolveService(context, 'protocol', envId); + // + // [#15238] Typed at the RESOLVE, not just at the callee's + // parameter. `resolveService` answers `any` for `'protocol'` (the + // slot is deliberately unmapped in `ServiceSlotContracts`), and + // this file was half-typed: {@link McpMergedMetadataRead} was + // already declared for the merged-read seam below, while the handle + // feeding it was annotated `any` — so any verb, spelt any way, + // could be reached from this site. Naming the existing type here + // closes that half. ⛔ Still `| undefined` and still every member + // optional: `readMergedSkillRows` keeps its own + // `typeof protocol.getMetaItems !== 'function'` probe, because a + // host may occupy the slot with a partial object. + const protocol: McpMergedMetadataRead | undefined = + await deps.resolveService(context, 'protocol', envId); return await readMergedSkillRows(deps, protocol, getMeta); }, diff --git a/packages/runtime/src/domains/meta.ts b/packages/runtime/src/domains/meta.ts index 33286fccd9..b2c9a06fb1 100644 --- a/packages/runtime/src/domains/meta.ts +++ b/packages/runtime/src/domains/meta.ts @@ -35,10 +35,108 @@ import { // org-scoped to the caller's own active organization. metaWriteCapabilityVerdict, } from '@objectstack/metadata-core'; +// [#15238] The DECLARED protocol contracts this domain's request literals are +// compiled against. Imported, never restated: a second hand-written +// `getMetaItem(…)` signature here would silently drift from the one the spec +// declares and `ObjectStackProtocolImplementation` states it `implements` — +// which is the whole reason `MetaDomainProtocol` below is `Pick`ed rather than +// written out. Same move `domains/packages.ts` and `domains/mcp.ts` make. +import type { MetadataProtocol } from '@objectstack/spec/api'; import { buildApiError } from '../error-envelope.js'; import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js'; import type { DomainHandlerDeps, DomainRoute } from '../domain-handler-registry.js'; +/** + * [#15238] The `protocol` service slot **as this domain reaches it** — one + * statement of the handle, replacing nine independent `protocol` seams in this + * file, four of which reached a verb through an `any` cast. + * + * ## What was wrong with the seam + * + * `deps.resolveService(context, 'protocol')` answers `any`. That is not an + * oversight — {@link DomainHandlerDeps.resolveService} types its return from + * `ServiceSlotContracts`, and `protocol` is deliberately left unmapped there + * ("real services with no written contract, so they keep today's `any` rather + * than being given a shape here that nothing verifies"). The `any` is honest + * about the SLOT. What it also did, silently, was hand every request literal + * downstream of it an unchecked call target: the #11006 series' end state — + * "an undeclared key in a request literal is a compile error" — stopped one + * seam short here, so a misspelt or undeclared key in these literals compiled, + * and so did a misspelt VERB. + * + * ## Why the type is here and not on the slot + * + * Mapping `'protocol'` in `ServiceSlotContracts` would type every consumer at + * once, but it is a `packages/spec` change that would state that a filled slot + * IS a `MetadataProtocol`, whose members are mostly REQUIRED — the shape the + * probes below exist to deny — and it would have to answer for the three verbs + * in the second group, which no contract declares at all. So the narrowing + * happens at the consumer, once, exactly as `domains/packages.ts` (#13598) and + * `domains/mcp.ts` (#8726) narrow the same slot for their own seams. + * + * ## ⛔ Every member is OPTIONAL, and the runtime probes STAY + * + * A host may occupy this slot with a partial object — that is the documented + * reason the `typeof protocol. === 'function'` probes exist, and every + * one of them survives this change unchanged in meaning. `Partial<…>` is what + * makes the type agree with them instead of contradicting them: tightening the + * type and then deleting a probe would trade a compile-time improvement for a + * runtime crash. The type answers "is this key declared?"; the probe answers + * "did THIS host bring the verb?". Two different questions, both still asked. + * + * ## Where the ledger honestly ends + * + * The first group names shapes someone DECLARES: the spec's `MetadataProtocol`, + * whose `GetMetaItemRequest` / `GetMetaItemsRequest` / `SaveMetaItemRequest` / + * `GetMetaItemLayeredRequest` are what this file's literals are now compiled + * against. The second group has no declared request shape anywhere: + * `@objectstack/metadata-protocol` types `listDrafts`, `migrateStoredMetadata` + * and `getProjectId` inline on the implementation class and exports nothing for + * them. Writing a structural request type for them HERE would be a private + * restatement that nothing verifies — the thing #9846 retired one file over. So + * their request keeps `any` and the gap stays visible and greppable: declaring + * them is producer-side work, not this consumer's to invent. What the entries + * still buy is the verb NAME — `protocol.migrateStoredMetadta` is now a compile + * error where the `any` handle took any spelling at all. + * + * `environmentId` is a PROPERTY, not a verb: the scope probe at the object-read + * branch reads it as the fallback for a host that brings no `getProjectId`. + * `unknown` rather than `string`, for the same reason the verbs above keep + * `any` requests — nothing declares its type, and the only thing that branch + * asks of it is whether it is `undefined`. + */ +export type MetaDomainProtocol = + Partial> + & { + /** ⚠️ Undeclared request shapes — see "Where the ledger honestly ends". */ + listDrafts?(request: any): Promise; + migrateStoredMetadata?(request: any): Promise; + getProjectId?(): unknown; + /** ⚠️ Undeclared PROPERTY — the `getProjectId` fallback, read for presence only. */ + environmentId?: unknown; + }; + +/** + * [#15238] Resolve the `protocol` slot as {@link MetaDomainProtocol}. + * + * THE one narrowing point for this file, mirroring `domains/packages.ts`'s + * `resolveProtocol`. `resolveService` answers `any` for this name, so the + * widening happens here and nowhere else — every call site downstream holds a + * typed handle, and a tenth call site added next month gets the type by + * construction rather than by remembering to write one. + * + * ⛔ Not a guard and not a replacement for one: it neither probes for verbs nor + * rejects a partial host. `undefined` still means "no protocol service", and + * each caller still asks its own `typeof … === 'function'` capability question. + */ +async function resolveProtocol( + deps: DomainHandlerDeps, + context: HttpProtocolContext, +): Promise { + return await deps.resolveService(context, 'protocol'); +} + /** * [#8848] The methods `/metadata/:type/:name` actually serves — the single * source for both the `Allow` header and the refusal message, so the two @@ -245,7 +343,7 @@ export async function handleMetadataRequest(deps: DomainHandlerDeps, path: strin // JSON Schemas, allowOrgOverride flags, domain, etc) needed by // the metadata admin UI. It internally also merges // MetadataService runtime types, so this path is strictly richer. - const protocol = await deps.resolveService(_context, 'protocol'); + const protocol = await resolveProtocol(deps, _context); if (protocol && typeof protocol.getMetaTypes === 'function') { try { const result = await protocol.getMetaTypes({}); @@ -339,11 +437,11 @@ export async function handleMetadataRequest(deps: DomainHandlerDeps, path: strin // code-published item resolving to the same bytes it always did. The // broader `getMetaItem` would not do: it folds the code layer into its // own answer, so this route could no longer tell the two stores apart. - const protocol = await deps.resolveService(_context, 'protocol'); - if (protocol && typeof (protocol as any).getMetaItemLayered === 'function') { + const protocol = await resolveProtocol(deps, _context); + if (protocol && typeof protocol.getMetaItemLayered === 'function') { try { const organizationId = await deps.resolveActiveOrganizationId(_context); - const layered = await (protocol as any).getMetaItemLayered({ + const layered = await protocol.getMetaItemLayered({ type, name, ...(organizationId ? { organizationId } : {}), @@ -494,7 +592,7 @@ export async function handleMetadataRequest(deps: DomainHandlerDeps, path: strin const item = body ?? {}; // Try to get the protocol service directly - const protocol = await deps.resolveService(_context, 'protocol'); + const protocol = await resolveProtocol(deps, _context); if (protocol && typeof protocol.saveMetaItem === 'function') { try { @@ -682,7 +780,7 @@ export async function handleMetadataRequest(deps: DomainHandlerDeps, path: strin // service (which filters sys_metadata by environment_id) in that // case, and fall back to the registry only for the // unscoped (single-kernel / control-plane) path. - const protocol = await deps.resolveService(_context, 'protocol') as any; + const protocol = await resolveProtocol(deps, _context); const scopedEnv = typeof protocol?.getProjectId === 'function' ? protocol.getProjectId() : protocol?.environmentId; @@ -697,7 +795,13 @@ export async function handleMetadataRequest(deps: DomainHandlerDeps, path: strin // very fan-out. const objectMasker = await resolveObjectMasker(deps, _context); - if (scoped && typeof protocol.getMetaItem === 'function') { + // [#15238] `protocol &&` spelled out, matching the `!scoped` twin + // below. Behaviour-identical: `scoped` is derived from + // `protocol?.getProjectId` / `protocol?.environmentId`, so it can only + // be true when the handle is there. The `any` cast this branch used to + // resolve through is what let the two sibling guards drift apart in + // spelling — typing the handle is what surfaced it (TS18048). + if (scoped && protocol && typeof protocol.getMetaItem === 'function') { try { const organizationId = await deps.resolveActiveOrganizationId(_context); const data = await protocol.getMetaItem({ type: 'object', name, organizationId }); @@ -757,7 +861,7 @@ export async function handleMetadataRequest(deps: DomainHandlerDeps, path: strin const singularType = pluralToSingular(type); // Try Protocol Service First (Preferred) - const protocol = await deps.resolveService(_context, 'protocol'); + const protocol = await resolveProtocol(deps, _context); if (protocol && typeof protocol.getMetaItem === 'function') { try { const organizationId = await deps.resolveActiveOrganizationId(_context); @@ -824,7 +928,7 @@ export async function handleMetadataRequest(deps: DomainHandlerDeps, path: strin ), }; } - const protocol = await deps.resolveService(_context, 'protocol'); + const protocol = await resolveProtocol(deps, _context); if (protocol && typeof protocol.listDrafts === 'function') { try { const organizationId = await deps.resolveActiveOrganizationId(_context); @@ -881,15 +985,15 @@ export async function handleMetadataRequest(deps: DomainHandlerDeps, path: strin }; } - const protocol = await deps.resolveService(_context, 'protocol'); - if (!protocol || typeof (protocol as any).migrateStoredMetadata !== 'function') { + const protocol = await resolveProtocol(deps, _context); + if (!protocol || typeof protocol.migrateStoredMetadata !== 'function') { return { handled: true, response: deps.error('Stored-metadata migration not supported', 501) }; } const types = Array.isArray(body?.types) ? body.types.filter((t: unknown): t is string => typeof t === 'string' && t.length > 0) : undefined; try { - const report = await (protocol as any).migrateStoredMetadata({ + const report = await protocol.migrateStoredMetadata({ apply: body?.apply === true, ...(types && types.length > 0 ? { types } : {}), // Attributed to the caller, not to the route: this writes @@ -910,7 +1014,7 @@ export async function handleMetadataRequest(deps: DomainHandlerDeps, path: strin const packageId = query?.package || undefined; // Try protocol service first for any type - const protocol = await deps.resolveService(_context, 'protocol'); + const protocol = await resolveProtocol(deps, _context); if (protocol && typeof protocol.getMetaItems === 'function') { try { const organizationId = await deps.resolveActiveOrganizationId(_context); @@ -987,7 +1091,7 @@ export async function handleMetadataRequest(deps: DomainHandlerDeps, path: strin if (parts.length === 0) { // Prefer protocol service for the rich `entries` array (with // JSON Schemas etc); fall back to MetadataService types-only. - const protocol = await deps.resolveService(_context, 'protocol'); + const protocol = await resolveProtocol(deps, _context); if (protocol && typeof protocol.getMetaTypes === 'function') { try { const result = await protocol.getMetaTypes({}); diff --git a/packages/runtime/src/domains/ui.ts b/packages/runtime/src/domains/ui.ts index 97d0a47737..c5baca7c22 100644 --- a/packages/runtime/src/domains/ui.ts +++ b/packages/runtime/src/domains/ui.ts @@ -8,10 +8,75 @@ * GET /view/:object[/:type] → getUiView (type also accepted as ?type=) */ +// [#15238] The DECLARED protocol contract this domain's ONE request literal is +// compiled against. Imported, never restated: a hand-written `getUiView(...)` +// signature here would silently drift from the one the spec declares and +// `ObjectStackProtocolImplementation` states it `implements` — which is the +// whole reason `UiDomainProtocol` below is `Pick`ed rather than written out. +// Same move `domains/packages.ts` (#13598) and `domains/mcp.ts` (#8726) make. +import type { MetadataProtocol } from '@objectstack/spec/api'; import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js'; import type { DomainHandlerDeps, DomainRoute } from '../domain-handler-registry.js'; import { capabilityUnavailable } from './unavailable.js'; +/** + * [#15238] The `protocol` service slot **as this domain reaches it** — one + * verb, `Pick`ed from the DECLARED contract. + * + * ## What was wrong with the seam + * + * `deps.resolveService(context, 'protocol')` answers `any`. That is not an + * oversight — {@link DomainHandlerDeps.resolveService} types its return from + * `ServiceSlotContracts`, and `protocol` is deliberately left unmapped there + * ("real services with no written contract, so they keep today's `any` rather + * than being given a shape here that nothing verifies"). The `any` is honest + * about the SLOT. What it also did, silently, was hand this domain's single + * request literal an unchecked call target: `getUiView` and its + * `GetUiViewRequest` are BOTH declared in `packages/spec`, and the one call + * below still compiled against nothing at all — neither the verb's spelling + * nor the two keys it sends. + * + * ## Why the type is here and not on the slot + * + * Mapping `'protocol'` in `ServiceSlotContracts` would type every consumer at + * once, but it is a `packages/spec` change that would state that a filled slot + * IS a `MetadataProtocol`, whose members are mostly REQUIRED — the shape the + * guard below exists to deny — and it would have to answer for the verbs no + * contract declares at all. So the narrowing happens at the consumer, once. + * + * ## ⛔ Every member is OPTIONAL, and the runtime guard STAYS + * + * A host may occupy this slot with a partial object — that is the documented + * reason the `typeof protocol.getUiView === 'function'` probe exists, and it + * survives this change unchanged in meaning. `Partial<…>` is what makes the + * type agree with the probe instead of contradicting it: tightening the type + * and then deleting the probe would trade a compile-time improvement for a + * runtime crash. The type answers "is this key declared?"; the probe answers + * "did THIS host bring the verb?". Two different questions, both still asked. + * `Partial` is not redundant with the spec's own `getUiView?`: it keeps the + * invariant true here even if the declaration upstream is ever tightened. + */ +export type UiDomainProtocol = Partial>; + +/** + * [#15238] Resolve the `protocol` slot as {@link UiDomainProtocol}. + * + * THE one narrowing point for this file, mirroring `domains/packages.ts`'s + * `resolveProtocol`. `resolveService` answers `any` for this name, so the + * widening happens here and nowhere else — a second call site added later gets + * the type by construction rather than by remembering to write one. + * + * ⛔ Not a guard and not a replacement for one: it neither probes for verbs nor + * rejects a partial host. `undefined` still means "no protocol service", and + * the caller still asks its own `typeof … === 'function'` capability question. + */ +async function resolveProtocol( + deps: DomainHandlerDeps, + context: HttpProtocolContext, +): Promise { + return await deps.resolveService(context, 'protocol'); +} + export function createUiDomain(deps: DomainHandlerDeps): DomainRoute { return { prefix: '/ui', @@ -35,7 +100,7 @@ export async function handleUiRequest( // Support both path param /view/obj/list AND query param /view/obj?type=list const type = parts[2] || query?.type || 'list'; - const protocol = await deps.resolveService(_context, 'protocol'); + const protocol = await resolveProtocol(deps, _context); if (protocol && typeof protocol.getUiView === 'function') { try {