From dd85dd1b46d5b88f45361232ea42435d6b5ef1de Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 11:14:09 +0000 Subject: [PATCH] refactor(metadata-protocol): move the artifact registry's lookups out of protocol.ts (#25467 cut 15) Move-only. The bodies of lookupArtifactItem, artifactLockLayerAt, shippedArtifactsOf and shippingPackagesOf move into the new protocol-artifact-lookup.ts (host shape B: a live engine getter, and protocol-bound closures for lookupArtifactItem, shippingPackagesOf and shippedArtifactsOf, so the moved bodies reach each other through the class methods). All four members keep their name, docblock and signature in the class as stubs that delegate. resolveArtifactLockLayer leaves protocol.ts' import with its last reader. Nothing re-exports the new module, so the published surface does not move. The #22024 call-site pin scans the new module beside protocol.ts: the lookup's body and shippedArtifactsOf's two calls of it are recorded there, the stub's host closure is recorded as a new site, and a non-vacuity case checks that the seam module's sites are found. Claude-Session: https://claude.ai/code/session_016SP1spj613tyGCgxoD1An6 Co-authored-by: Claude --- .../src/protocol-artifact-lookup.ts | 158 ++++++++++++++++++ ...ol.lookup-artifact-item-call-sites.test.ts | 66 ++++++-- packages/metadata-protocol/src/protocol.ts | 94 +++-------- 3 files changed, 231 insertions(+), 87 deletions(-) create mode 100644 packages/metadata-protocol/src/protocol-artifact-lookup.ts diff --git a/packages/metadata-protocol/src/protocol-artifact-lookup.ts b/packages/metadata-protocol/src/protocol-artifact-lookup.ts new file mode 100644 index 00000000000..3a12cec98c9 --- /dev/null +++ b/packages/metadata-protocol/src/protocol-artifact-lookup.ts @@ -0,0 +1,158 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#25467 cut 15] The artifact registry's lookups of + * `ObjectStackProtocolImplementation` (ADR-0010 metadata protection): the + * artifact an installed code package registered for an item + * (`lookupArtifactItem`), every package's artifact for it + * (`shippedArtifactsOf`), the packages that can ship an item of a type + * (`shippingPackagesOf`), and the item-lock resolution's artifact layer over + * them (`artifactLockLayerAt`), moved out of `protocol.ts` whole. + * + * A MOVE, not a redesign. Each function below is the body of the + * `ObjectStackProtocolImplementation` method of the same name as it stood, + * re-indented one level and otherwise unedited: the same registry reads, in + * the same order, answering the same artifacts. + * + * ## How the moved bodies reach the class + * + * The shape #23050 set for `rest-server.ts` (its `data-action-routes.ts` + * header carries the full reasoning), as `protocol-batch-data.ts` applies it. + * The class keeps every method — the published `dist/*.d.ts` lists each one's + * name, docblock and declared signature — and each body is one literal of + * exactly the {@link ArtifactLookupHost} members that method reads, plus a + * `.call`. The engine is handed over as a getter, so the body reads the + * protocol's engine at call time. A method is handed over as a closure that + * calls it on the protocol at call time, so a test double or spy set on the + * instance is still the one the body reaches. That holds between the four + * functions here too: `shippedArtifactsOf` reaches the lookup and the + * shipping-package read through the class methods, not by calling the + * functions below directly. Every other reader of these four methods keeps + * calling the class method. + * + * #22024's call-site pin (`protocol.lookup-artifact-item-call-sites.test.ts`) + * scans this module beside `protocol.ts`: the lookup's definition and + * `shippedArtifactsOf`'s two calls of it live here now. + * + * What the bodies read at module level they import from where `protocol.ts` + * imported it; `ShippingPackages` is `protocol.ts`'s own exported type. + * Nothing re-exports this module, so the published surface does not move. + */ + +import { PLURAL_TO_SINGULAR } from '@objectstack/spec/shared'; +import { resolveArtifactLockLayer, type ItemAddress } from './item-lock.js'; +import type { ObjectStackProtocolImplementation, ShippingPackages } from './protocol.js'; + +/** + * The `ObjectStackProtocolImplementation` members the functions below read — + * every one of them, and no other; each function's `this` is the `Pick` of the + * members IT reads. Each is typed AS the class member (an indexed access on + * the class), so a change to a member's signature on the class is a change + * here too, and naming a member the class does not have is a compile error. + */ +export interface ArtifactLookupHost { + readonly engine: ObjectStackProtocolImplementation['engine']; + lookupArtifactItem: ObjectStackProtocolImplementation['lookupArtifactItem']; + shippingPackagesOf: ObjectStackProtocolImplementation['shippingPackagesOf']; + shippedArtifactsOf: ObjectStackProtocolImplementation['shippedArtifactsOf']; +} + +/** The body of `ObjectStackProtocolImplementation#lookupArtifactItem`; the method's docblock there is its contract. */ +export function lookupArtifactItem( + this: Pick, + type: string, name: string, currentPackageId?: string, member?: string, +): unknown { + const registry = (this.engine as any)?.registry; + if (!registry) return undefined; + const singular = PLURAL_TO_SINGULAR[type] ?? type; + // Prefer the artifact-only lookup: it scans composite + // (`:`) entries first, so an overlay row hydrated + // into the plain key (getMetaItems / loadMetaFromDb) can never + // shadow the packaged artifact's protection envelope (ADR-0010 + // §3.3 — pre-fix, that shadow made a `_lock: full` app read back + // as unlocked after PUT+GET until restart). `currentPackageId` + // (ADR-0048) makes that scan package-scoped (prefer-local). + if (typeof registry.getArtifactItem === 'function') { + return registry.getArtifactItem(singular, name, currentPackageId, member) + ?? registry.getArtifactItem(type, name, currentPackageId, member); + } + // Partial registry mocks in tests — fall back to getItem and apply + // the same package-provenance filter inline. + if (typeof registry.getItem !== 'function') return undefined; + const item = registry.getItem(singular, name, currentPackageId) ?? registry.getItem(type, name, currentPackageId); + if (!item || !(item as any)._packageId || (item as any)._packageId === 'sys_metadata') { + return undefined; + } + return item; +} + +/** The body of `ObjectStackProtocolImplementation#artifactLockLayerAt`; the method's docblock there is its contract. */ +export function artifactLockLayerAt( + this: Pick, + address: ItemAddress, shipping?: ShippingPackages, +): readonly unknown[] { + return resolveArtifactLockLayer(address, () => this.shippedArtifactsOf(address.type, address.name, shipping)); +} + +/** The body of `ObjectStackProtocolImplementation#shippedArtifactsOf`; the method's docblock there is its contract. */ +export function shippedArtifactsOf( + this: Pick, + type: string, name: string, shipping?: ShippingPackages, +): unknown[] { + const shipped: unknown[] = []; + const add = (artifact: unknown): void => { + if (artifact !== undefined && artifact !== null && !shipped.includes(artifact)) shipped.push(artifact); + }; + add(this.lookupArtifactItem(type, name)); + if ((PLURAL_TO_SINGULAR[type] ?? type) === 'object') return shipped; + const packages = shipping ?? this.shippingPackagesOf(type); + for (const packageId of new Set([...(packages.byName.get(name) ?? []), ...packages.anyName])) { + const own = this.lookupArtifactItem(type, name, packageId) as { _packageId?: unknown } | undefined; + if (own?._packageId === packageId) add(own); + } + return shipped; +} + +/** The body of `ObjectStackProtocolImplementation#shippingPackagesOf`; the method's docblock there is its contract. */ +export function shippingPackagesOf( + this: Pick, + type: string, +): ShippingPackages { + const registry = (this.engine as any)?.registry; + const byName = new Map>(); + const anyName = new Set(); + if (!registry) return { byName, anyName }; + const isPackage = (id: unknown): id is string => typeof id === 'string' && id !== '' && id !== 'sys_metadata'; + const listing = (read: () => unknown): readonly unknown[] => { + try { + const listed = read(); + return Array.isArray(listed) ? listed : []; + } catch { + return []; // See this method's header: the package-less lookup still answers. + } + }; + if (typeof registry.listItems === 'function') { + for (const spelling of new Set([PLURAL_TO_SINGULAR[type] ?? type, type])) { + for (const entry of listing(() => registry.listItems(spelling))) { + const { _packageId: id, name } = (entry ?? {}) as { _packageId?: unknown; name?: unknown }; + if (!isPackage(id)) continue; + if (typeof name !== 'string') { + anyName.add(id); + continue; + } + const ids = byName.get(name); + if (ids) ids.add(id); + else byName.set(name, new Set([id])); + } + } + } + if (typeof registry.getAllPackages === 'function') { + const canTell = typeof registry.isPackageDisabled === 'function'; + for (const record of listing(() => registry.getAllPackages())) { + const r = record as { manifest?: { id?: unknown }; id?: unknown } | null | undefined; + const id = r?.manifest?.id ?? r?.id; + if (isPackage(id) && (!canTell || registry.isPackageDisabled(id))) anyName.add(id); + } + } + return { byName, anyName }; +} diff --git a/packages/metadata-protocol/src/protocol.lookup-artifact-item-call-sites.test.ts b/packages/metadata-protocol/src/protocol.lookup-artifact-item-call-sites.test.ts index f95958233ff..5a95dbdcfd4 100644 --- a/packages/metadata-protocol/src/protocol.lookup-artifact-item-call-sites.test.ts +++ b/packages/metadata-protocol/src/protocol.lookup-artifact-item-call-sites.test.ts @@ -22,26 +22,46 @@ * * The population is the card's own enumeration command, * `git grep -n 'lookupArtifactItem(' -- packages/metadata-protocol/src` - * (tests excluded), which is one file: every match is a call or the - * definition. Each is keyed by the class member it sits in and its argument - * text, which survive line moves and fail on any change of scope. A new call, - * a removed one or a re-scoped one turns this red until its row is recorded - * here with its disposition (and the card's table in the PR that adds it). + * (tests excluded), which is two files: every match is a call or a + * definition. Each is keyed by the file, the class member or module function + * it sits in, and its argument text, which survive line moves and fail on any + * change of scope. A new call, a removed one or a re-scoped one turns this red + * until its row is recorded here with its disposition (and the card's table in + * the PR that adds it). + * + * [#25467 cut 15] The second file is `protocol-artifact-lookup.ts`. The + * lookup's body moved there with `shippedArtifactsOf`'s, so it holds the + * function the class member delegates to and that member's two calls; the + * class member keeps its signature in `protocol.ts`, and `shippedArtifactsOf`'s + * stub there hands the moved body the lookup as a closure, one more site. */ import { readFileSync } from 'node:fs'; import { fileURLToPath } from 'node:url'; import { describe, expect, it } from 'vitest'; const SOURCE = readFileSync(fileURLToPath(new URL('./protocol.ts', import.meta.url)), 'utf8'); +/** [#25467 cut 15] The seam module the lookup's body moved to; scanned with its own header shape. */ +const SEAM_FILE = 'protocol-artifact-lookup.ts'; +const SEAM = readFileSync(fileURLToPath(new URL(`./${SEAM_FILE}`, import.meta.url)), 'utf8'); const CALLEE = 'lookupArtifactItem('; /** A class member's header in `protocol.ts`: four spaces, optional modifiers, its name, its parameter list. */ const MEMBER_HEADER = /^ {4}(?:(?:private|public|protected|static|async|override|readonly)\s+)*([A-Za-z_$][\w$]*)\s*(?:<[^>]*>)?\(/; +/** A module function's header in the seam module: `export`, optional `async`, `function`, its name, its parameter list. */ +const FUNCTION_HEADER = /^export (?:async )?function ([A-Za-z_$][\w$]*)\s*(?:<[^>]*>)?\(/; + +interface Site { readonly file?: string; readonly member: string; readonly args: string } -interface Site { readonly member: string; readonly args: string } +/** Every `lookupArtifactItem(` in `protocol.ts` and the seam module: the file, the member it sits in and its argument text, whitespace-normalised. */ +function sitesIn(): Site[] { + return [ + ...sitesOf(SOURCE, MEMBER_HEADER), + ...sitesOf(SEAM, FUNCTION_HEADER).map((site) => ({ ...site, file: SEAM_FILE })), + ]; +} -/** Every `lookupArtifactItem(` in the source: the member it sits in and its argument text, whitespace-normalised. */ -function sitesOf(source: string): Site[] { +/** Every `lookupArtifactItem(` in one source: the member it sits in (by `header`) and its argument text, whitespace-normalised. */ +function sitesOf(source: string, header: RegExp): Site[] { const lines = source.split('\n'); const sites: Site[] = []; for (let at = source.indexOf(CALLEE); at !== -1; at = source.indexOf(CALLEE, at + 1)) { @@ -53,8 +73,8 @@ function sitesOf(source: string): Site[] { const args = source.slice(at + CALLEE.length, end - 1).replace(/\s+/g, ' ').trim().replace(/,$/, ''); let member = '(no member)'; for (let line = source.slice(0, at).split('\n').length - 1; line >= 0; line--) { - const header = MEMBER_HEADER.exec(lines[line]); - if (header) { member = header[1]; break; } + const found = header.exec(lines[line]); + if (found) { member = found[1]; break; } } sites.push({ member, args }); } @@ -70,7 +90,9 @@ type Disposition = 'definition' | 'served-package envelope' | 'no'; */ const SITES: ReadonlyArray = [ { member: 'lookupArtifactItem', args: 'type: string, name: string, currentPackageId?: string, member?: string', noPackage: false, disposition: 'definition', - why: 'the definition itself' }, + why: 'the definition itself: the class member\'s declared signature (its body delegates to the seam function below)' }, + { file: SEAM_FILE, member: 'lookupArtifactItem', args: 'this: Pick, type: string, name: string, currentPackageId?: string, member?: string', noPackage: false, disposition: 'definition', + why: '[#25467 cut 15] the definition\'s body, the function the class member delegates to' }, { member: 'readMetaItem', args: 'request.type, request.name, envelopePackageId(request.packageId, item), bundleMemberOf(request.type, item)', noPackage: true, disposition: 'served-package envelope', why: 'the by-name read\'s envelope: naming no package, the package of the item it serves (the list\'s rule); [#23074] at the served item\'s own bundle member' }, { member: 'readFlattenedMetaItems', args: 'request.type, itemName, itemPackageId, bundleMemberOf(request.type, it)', noPackage: true, disposition: 'served-package envelope', @@ -91,10 +113,12 @@ const SITES: ReadonlyArray `${site.member}: ${CALLEE}${site.args})`; +const keyOf = (site: Site) => `${site.file ?? 'protocol.ts'} ${site.member}: ${CALLEE}${site.args})`; describe('[#22024] every lookupArtifactItem( in protocol.ts has a recorded disposition', () => { - const found = sitesOf(SOURCE); + const found = sitesIn(); it('the sites in the source are exactly the recorded ones', () => { const recorded = SITES.map(keyOf).sort(); @@ -128,11 +152,21 @@ describe('[#22024] every lookupArtifactItem( in protocol.ts has a recorded dispo it('the population is the card\'s enumeration: one definition and every call, each in a member', () => { // Non-vacuity: a scan that found nothing, or lost the members, cannot pass the test above by accident. - expect(found.filter((site) => site.member === 'lookupArtifactItem' && site.args.includes(': string'))).toHaveLength(1); + // [#25467 cut 15] One definition per file: the class member's signature and the seam function it delegates to. + expect(found.filter((site) => site.member === 'lookupArtifactItem' && site.args.includes(': string')).map((site) => site.file ?? 'protocol.ts')) + .toEqual(['protocol.ts', SEAM_FILE]); expect(found.filter((site) => site.member === '(no member)')).toEqual([]); expect(found.length).toBe(SITES.length); }); + it('[#25467 cut 15] the seam module is scanned: the definition\'s body and both of shippedArtifactsOf\'s calls are found there', () => { + // Non-vacuity for the second file: a scan that read the wrong file, or none, finds no site there. + expect(found.filter((site) => site.file === SEAM_FILE).map(keyOf).sort()).toEqual( + SITES.filter((site) => site.file === SEAM_FILE).map(keyOf).sort(), + ); + expect(found.filter((site) => site.file === SEAM_FILE)).toHaveLength(3); + }); + it('a site passing fewer than three arguments is recorded as passing no package', () => { const topLevelArgs = (args: string) => { let depth = 0; diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 727252769c7..5daf1011ba2 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -60,7 +60,6 @@ import { } from './sys-metadata-repository.js'; import { managedItemSealedSentence, packagedBaseRegimeRow, packagedBaseRegimeSentence } from './packaged-base-regime.js'; import { - resolveArtifactLockLayer, resolveItemLock, resolveItemLockLazily, resolveOverlayLockLayer, @@ -399,6 +398,8 @@ import { discardDraftInItsScope, listDrafts } from './protocol-drafts.js'; import { originGatedRemovalRefusal, refusePackagedBaseOverride } from './protocol-packaged-base-refusal.js'; // [#25467 cut 14] The publish batch's seed apply and its own pending declarations. import { applySeedBodies, collectBatchPendingDeclarations } from './protocol-publish-batch.js'; +// [#25467 cut 15] The artifact registry's lookups: an item's artifact, the packages that ship a type, and the lock's artifact layer. +import { artifactLockLayerAt, lookupArtifactItem, shippedArtifactsOf, shippingPackagesOf } from './protocol-artifact-lookup.js'; /** * Canonical Zod schema per metadata type lives in @@ -16491,28 +16492,12 @@ export class ObjectStackProtocolImplementation implements * ignores it, so a caller that needs the member checks the answer. */ private lookupArtifactItem(type: string, name: string, currentPackageId?: string, member?: string): unknown { - const registry = (this.engine as any)?.registry; - if (!registry) return undefined; - const singular = PLURAL_TO_SINGULAR[type] ?? type; - // Prefer the artifact-only lookup: it scans composite - // (`:`) entries first, so an overlay row hydrated - // into the plain key (getMetaItems / loadMetaFromDb) can never - // shadow the packaged artifact's protection envelope (ADR-0010 - // §3.3 — pre-fix, that shadow made a `_lock: full` app read back - // as unlocked after PUT+GET until restart). `currentPackageId` - // (ADR-0048) makes that scan package-scoped (prefer-local). - if (typeof registry.getArtifactItem === 'function') { - return registry.getArtifactItem(singular, name, currentPackageId, member) - ?? registry.getArtifactItem(type, name, currentPackageId, member); - } - // Partial registry mocks in tests — fall back to getItem and apply - // the same package-provenance filter inline. - if (typeof registry.getItem !== 'function') return undefined; - const item = registry.getItem(singular, name, currentPackageId) ?? registry.getItem(type, name, currentPackageId); - if (!item || !(item as any)._packageId || (item as any)._packageId === 'sys_metadata') { - return undefined; - } - return item; + // [#25467 cut 15] The body moved to `protocol-artifact-lookup.ts` (see `ArtifactLookupHost`). + const readEngine = () => this.engine; + const host: ThisParameterType = { + get engine() { return readEngine(); }, + }; + return lookupArtifactItem.call(host, type, name, currentPackageId, member); } /** @@ -16531,7 +16516,11 @@ export class ObjectStackProtocolImplementation implements * caller that asks about many items of one type. */ private artifactLockLayerAt(address: ItemAddress, shipping?: ShippingPackages): readonly unknown[] { - return resolveArtifactLockLayer(address, () => this.shippedArtifactsOf(address.type, address.name, shipping)); + // [#25467 cut 15] The body moved to `protocol-artifact-lookup.ts` (see `ArtifactLookupHost`). + const host: ThisParameterType = { + shippedArtifactsOf: (...a) => this.shippedArtifactsOf(...a), + }; + return artifactLockLayerAt.call(host, address, shipping); } /** @@ -16556,18 +16545,12 @@ export class ObjectStackProtocolImplementation implements * owner is the whole set. */ private shippedArtifactsOf(type: string, name: string, shipping?: ShippingPackages): unknown[] { - const shipped: unknown[] = []; - const add = (artifact: unknown): void => { - if (artifact !== undefined && artifact !== null && !shipped.includes(artifact)) shipped.push(artifact); + // [#25467 cut 15] The body moved to `protocol-artifact-lookup.ts` (see `ArtifactLookupHost`). + const host: ThisParameterType = { + lookupArtifactItem: (...a) => this.lookupArtifactItem(...a), + shippingPackagesOf: (...a) => this.shippingPackagesOf(...a), }; - add(this.lookupArtifactItem(type, name)); - if ((PLURAL_TO_SINGULAR[type] ?? type) === 'object') return shipped; - const packages = shipping ?? this.shippingPackagesOf(type); - for (const packageId of new Set([...(packages.byName.get(name) ?? []), ...packages.anyName])) { - const own = this.lookupArtifactItem(type, name, packageId) as { _packageId?: unknown } | undefined; - if (own?._packageId === packageId) add(own); - } - return shipped; + return shippedArtifactsOf.call(host, type, name, shipping); } /** @@ -16592,43 +16575,12 @@ export class ObjectStackProtocolImplementation implements * #21803, which asked that lookup alone. */ private shippingPackagesOf(type: string): ShippingPackages { - const registry = (this.engine as any)?.registry; - const byName = new Map>(); - const anyName = new Set(); - if (!registry) return { byName, anyName }; - const isPackage = (id: unknown): id is string => typeof id === 'string' && id !== '' && id !== 'sys_metadata'; - const listing = (read: () => unknown): readonly unknown[] => { - try { - const listed = read(); - return Array.isArray(listed) ? listed : []; - } catch { - return []; // See this method's header: the package-less lookup still answers. - } + // [#25467 cut 15] The body moved to `protocol-artifact-lookup.ts` (see `ArtifactLookupHost`). + const readEngine = () => this.engine; + const host: ThisParameterType = { + get engine() { return readEngine(); }, }; - if (typeof registry.listItems === 'function') { - for (const spelling of new Set([PLURAL_TO_SINGULAR[type] ?? type, type])) { - for (const entry of listing(() => registry.listItems(spelling))) { - const { _packageId: id, name } = (entry ?? {}) as { _packageId?: unknown; name?: unknown }; - if (!isPackage(id)) continue; - if (typeof name !== 'string') { - anyName.add(id); - continue; - } - const ids = byName.get(name); - if (ids) ids.add(id); - else byName.set(name, new Set([id])); - } - } - } - if (typeof registry.getAllPackages === 'function') { - const canTell = typeof registry.isPackageDisabled === 'function'; - for (const record of listing(() => registry.getAllPackages())) { - const r = record as { manifest?: { id?: unknown }; id?: unknown } | null | undefined; - const id = r?.manifest?.id ?? r?.id; - if (isPackage(id) && (!canTell || registry.isPackageDisabled(id))) anyName.add(id); - } - } - return { byName, anyName }; + return shippingPackagesOf.call(host, type); } /**