From 7a770b1ffafadd46163f10182c780f0a893561f0 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 21:44:20 +0000 Subject: [PATCH 1/3] fix(scripts): codemod-split-engine --members reads a hand-written header tail and inverts the import edits a move made The fourth pass refused engine-registration.ts, whose header a hand edit wrote on after (isExternalWriteForbidden), and failed the undo proof of registerHook, whose import of HookEntry resolves a docblock link the base module left unresolved, so the prune could not take it back. headerNamesOf now reads the hand's words after the header sentence and accepts the header when the names and those words rewrap to it byte for byte; the pass carries them over. The undo inverts the import edits it reads off the base and head import blocks (bindings added or widened, each read by a moved member; none dropped or narrowed) instead of relying on the prune alone. The comparison stays byte for byte. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_016WV4cwSsWgu5WEWLNtFrf3 --- scripts/codemod-split-engine.mjs | 172 +++++++++++++++++++++++++------ 1 file changed, 143 insertions(+), 29 deletions(-) diff --git a/scripts/codemod-split-engine.mjs b/scripts/codemod-split-engine.mjs index f7ab3b0b050..e055b2d82d9 100644 --- a/scripts/codemod-split-engine.mjs +++ b/scripts/codemod-split-engine.mjs @@ -1399,12 +1399,21 @@ function moduleText(moduleFile, title, movers, scope, leafNames, routed = false) } /** The opening lines of a facet module's header docblock: its section's title and the names of its bodies. */ -function moduleHeaderLines(title, names) { +/** + * A facet module's header sentence, wrapped. `tail` is the words a hand edit + * wrote after that sentence in the same paragraph (the fourth pass reads them + * back with `headerNamesOf` and carries them over unedited); the first pass + * writes none. + */ +function moduleHeaderLines(title, names, tail = []) { const listed = names.map((n, i) => `\`${n}\`${i === names.length - 1 ? '' : ','}`); return wrapComment([MARK, 'The', `"${title}"`, 'section', 'of', '`ObjectQL`:', 'the', 'bodies', 'of', - ...listed, 'moved', 'out', 'of', '`engine.ts`', 'whole', 'by', `\`${SCRIPT}\`.`]); + ...listed, ...HEADER_SENTENCE_END.trim().split(' '), ...tail]); } +/** The words that close a facet module's header sentence, after its list of names. */ +const HEADER_SENTENCE_END = ` moved out of \`engine.ts\` whole by \`${SCRIPT}\`.`; + /** * One facet member, as its module holds it: the method's docblock (a private * one) or the placeholder, `static`, and the method's own parameters, return @@ -3114,14 +3123,23 @@ async function mainHandDocs(tm, argv) { // decision someone writes here. // // The proof (`--members --verify --base REF` re-runs it) undoes the moves from -// the head alone, as the first pass's does: each moved body, docblock and -// `async` goes back onto its stub; each receiving module loses the moved -// members, the host members and import bindings only they read, and its header -// their names; the statics route and the host factory lose what only they -// read. The result must be the base `engine.ts` and modules byte for byte (the -// base's import bindings only moved bodies read pruned from it, as the first -// pass prunes them), every other generated module unchanged, and each moved -// member its base method's text exactly, by a stay-rule reading of the base. +// the head, as the first pass's does: each moved body, docblock and `async` +// goes back onto its stub; each receiving module loses the moved members, the +// host members and import bindings only they read, and its header their names; +// the statics route and the host factory lose what only they read. The result +// must be the base `engine.ts` and modules byte for byte (the base's import +// bindings only moved bodies read pruned from it, as the first pass prunes +// them), every other generated module unchanged, and each moved member its +// base method's text exactly, by a stay-rule reading of the base. +// +// Two shapes a hand edit leaves are read, not refused (#30572). A header the +// hand wrote on after, in its first paragraph, keeps those words after the +// sentence this pass rewrites (`headerNamesOf`), when the two rewrap to the +// header byte for byte. And the one import edit a read of the head cannot tell +// back, a binding a moved body brought that something staying still reads (a +// docblock link the base left unresolved), is inverted from the base's import +// block (`importEditsOf`): the bindings the head added or widened, each one a +// moved member reads, and none the base had dropped or narrowed. const MEMBERS_CARD = '#30055'; @@ -3202,16 +3220,27 @@ function headerRegionOf(text) { * is `moduleHeaderLines` of them. The list is the header's own, which a hand * edit to the module may have left behind its members: this pass adds and * removes its names in it and leaves the rest as it stands. + * + * A hand edit may also have written on after the header's sentence, in the + * same paragraph, about a member it wrote into the module (#30572: one the + * first pass never moved, so the list does not name it). Those words come back + * as `tail`, and the header is accepted only when `moduleHeaderLines` of the + * names and that tail rewraps to it byte for byte: what the pass rewrites is + * then exactly the sentence it owns, with the hand's words after it carried + * over, and the undo rewraps the same tail. */ function headerNamesOf(text, title) { const region = headerRegionOf(text); if (!region) return null; const prose = text.slice(region.start, region.end).split('\n').map((l) => l.replace(/^ \*( |$)/, '')).join(' '); const i = prose.indexOf(' the bodies of '); - const j = prose.lastIndexOf(' moved out of '); - if (i < 0 || j < i) return null; + const j = i < 0 ? -1 : prose.indexOf(HEADER_SENTENCE_END, i); + if (j < 0) return null; const names = [...prose.slice(i, j).matchAll(/`([^`]+)`/g)].map((m) => m[1]); - return moduleHeaderLines(title, names).join('\n') === text.slice(region.start, region.end) ? { region, names } : null; + const rest = prose.slice(j + HEADER_SENTENCE_END.length); + const tail = rest === '' ? [] : rest.startsWith(' ') ? rest.slice(1).split(' ') : null; + if (!tail || tail.some((w) => w === '')) return null; + return moduleHeaderLines(title, names, tail).join('\n') === text.slice(region.start, region.end) ? { region, names, tail } : null; } /** A facet module's host interface: its member lines, each `[readonly ]m: Class['m'];`, in order. */ @@ -3328,9 +3357,12 @@ function specifierLines(specifier, bindings) { /** * Drop the import bindings nothing in a facet module reads, re-rendering each * specifier's lines as the first pass writes them (`specifierLines`), so a - * pruned leaf list wraps as `namedStatement` wraps it. + * pruned leaf list wraps as `namedStatement` wraps it. `inverse` is the import + * edits a move made that a read cannot tell back (`importEditsOf`): the + * bindings it added are dropped though something left still reads them, and + * the ones it widened from type-only to value are made type-only again. */ -function pruneModuleImports(tm, moduleFile, text) { +function pruneModuleImports(tm, moduleFile, text, inverse = { added: new Set(), widened: new Set() }) { const used = usedImportNames(tm, text); const decls = importDeclsOf(tm, moduleFile, text); const edits = []; @@ -3338,8 +3370,11 @@ function pruneModuleImports(tm, moduleFile, text) { const run = decls.filter((d) => d.specifier === specifier); const have = new Map(); for (const d of run) for (const [local, e] of d.bindings) have.set(local, e); - const kept = new Map([...have].filter(([local]) => used.has(local))); - if (kept.size === have.size) continue; + const key = (local) => importKey(specifier, local); + const kept = new Map([...have] + .filter(([local]) => used.has(local) && !inverse.added.has(key(local))) + .map(([local, e]) => [local, inverse.widened.has(key(local)) ? { ...e, value: false } : e])); + if (kept.size === have.size && [...kept].every(([local, e]) => e.value === have.get(local).value)) continue; const start = run[0].start; if (kept.size === 0) { const end = text[run.at(-1).end] === '\n' ? run.at(-1).end + 1 : run.at(-1).end; @@ -3502,7 +3537,7 @@ function generateMembers(tm, text, moduleTexts, planned) { const k = listed.findIndex((n) => classPos(n) > r.node.getStart()); listed.splice(k < 0 ? listed.length : k, 0, r.name); } - modEdits.push({ start: header.region.start, end: header.region.end, text: moduleHeaderLines(title, listed).join('\n') }); + modEdits.push({ start: header.region.start, end: header.region.end, text: moduleHeaderLines(title, listed, header.tail).join('\n') }); // The host interface gains what the moved bodies read. const reads = new Map(); const writes = new Set(); @@ -3580,15 +3615,55 @@ function generateMembers(tm, text, moduleTexts, planned) { return { host: pruned.text, modules: moduleOut, pruned: pruned.removed }; } +/** One import binding's identity in a module: its specifier and its local name. */ +const importKey = (specifier, local) => `${specifier}\u0000${local}`; + +/** + * The import edits this pass made to a receiving module, read off its base + * and head import blocks (#30572): the bindings the head has and the base had + * not (`added`), and the ones the base bound type-only and the head binds as a + * value (`widened`). The prune alone cannot undo either when the module's + * remaining text still reads the name: a docblock link the base carried + * unresolved resolves once a moved body's import binds it, and a binding a + * staying member reads as a type reads as one still. So the proof inverts the + * edits it reads here instead, and each is checked on the way (`fail`): it is + * one the moved members' own text reads (a moved body, or its docblock), and + * the head never narrowed or dropped a base binding, which no undo could + * restore. The comparison stays byte for byte: the edits name bindings only, + * and the lines they leave are rendered as the first pass writes them. + */ +function importEditsOf(tm, file, baseText, headText, movedText, fail) { + const bindingsOf = (text) => { + const out = new Map(); + for (const d of importDeclsOf(tm, file, text)) for (const [local, e] of d.bindings) out.set(importKey(d.specifier, local), { local, ...e }); + return out; + }; + const base = bindingsOf(baseText); + const head = bindingsOf(headText); + const added = new Set(); + const widened = new Set(); + const readByMoved = (local) => new RegExp(`\\b${local.replace(/\$/g, '\\$')}\\b`).test(movedText); + for (const [k, h] of head) { + const b = base.get(k); + if (b && b.value === h.value) continue; + if (b && b.value && !h.value) { fail(`${file}: binds ${h.local} type-only, where its base bound it as a value`); continue; } + if (!readByMoved(h.local)) { fail(`${file}: ${b ? 'binds as a value' : 'imports'} ${h.local}, which ${b ? 'its base bound type-only' : 'its base did not'} and no moved member reads`); continue; } + (b ? widened : added).add(k); + } + for (const [k, b] of base) if (!head.has(k)) fail(`${file}: dropped the import binding ${b.local}, which no undo can restore`); + return { added, widened }; +} + /** - * Undo this pass's moves out of one receiving module, from its head text - * alone: drop the named members and their names from the header's list, keep - * the host members the rest still `Pick`, and prune the imports nothing reads. - * Checks the head on the way (`fail`): the header lists them, the interface is the - * union of the `Pick`s, and no import binding is unread. Returns the text, or - * null when the head is not a module this pass can read. + * Undo this pass's moves out of one receiving module, from its head text and + * its base's import block: drop the named members and their names from the + * header's list, keep the host members the rest still `Pick`, invert the + * import edits the moves made (`importEditsOf`) and prune the imports nothing + * reads. Checks the head on the way (`fail`): the header lists them, the + * interface is the union of the `Pick`s, and no import binding is unread. + * Returns the text, or null when the head is not a module this pass can read. */ -function undoMembersModule(tm, file, text, names, title, fail) { +function undoMembersModule(tm, file, text, names, title, fail, baseText) { const facet = facetMembersOf(tm, file, text); const header = headerNamesOf(text, title); if (!facet || !header) { fail(`${file}: not a facet module as the first pass writes one`); return null; } @@ -3601,15 +3676,17 @@ function undoMembersModule(tm, file, text, names, title, fail) { if (unread.length > 0) fail(`${file}: imports what nothing in it reads: ${unread.join(', ')}`); const keep = facet.members.filter((m) => !names.includes(m.name)); if (keep.length === 0) { fail(`${file}: every member is one this pass moved (it never starts a facet)`); return null; } + const movedText = facet.members.filter((m) => names.includes(m.name)).map((m) => m.text).join('\n'); + const inverse = importEditsOf(tm, file, baseText, text, movedText, fail); const edits = [ { start: facet.start, end: facet.end, text: keep.map((m) => m.text).join('\n\n') }, - { start: header.region.start, end: header.region.end, text: moduleHeaderLines(title, header.names.filter((n) => !names.includes(n))).join('\n') }, + { start: header.region.start, end: header.region.end, text: moduleHeaderLines(title, header.names.filter((n) => !names.includes(n)), header.tail).join('\n') }, ]; if (iface) { const kept = union(keep); edits.push({ start: iface.start, end: iface.end, text: [...iface.members].filter(([k]) => kept.has(k)).map(([, v]) => v.line).join('\n') }); } - return pruneModuleImports(tm, file, applyOrderedEdits(text, edits)); + return pruneModuleImports(tm, file, applyOrderedEdits(text, edits), inverse); } /** The statics `ObjectQL.x` each routed module's text reads. */ @@ -3691,7 +3768,7 @@ function verifyMembers(tm, baseText, headText, baseModules, headModules, { } if (b === null) { fail(`${file}: absent at base (this pass never starts a facet)`); continue; } const mvs = touched.get(file); - const u = undoMembersModule(tm, file, h, mvs.map((mv) => mv.name), mvs[0].title, fail); + const u = undoMembersModule(tm, file, h, mvs.map((mv) => mv.name), mvs[0].title, fail, b); if (u === null) continue; undone.set(file, u); if (u !== b) fail(`undoing the moves out of ${file} does not rebuild its base (first difference at line ${firstDiffLine(u, b)})`); @@ -3929,7 +4006,7 @@ const SELF_TEST_BATTERIES = Object.freeze({ 'refusals': 6, 'scoped classes (second pass)': 16, 'hand-series docblocks (third pass)': 19, - 'named methods onto the split class (fourth pass)': 18, + 'named methods onto the split class (fourth pass)': 23, }); const SELF_TEST_BATTERY_FLOOR = 5; @@ -4646,6 +4723,43 @@ async function selfTest() { fails(out.host.replace(' return 2 + VIA_SEAM', ' return 3 + VIA_SEAM'), []) && fails(out.host, [[LEAF_MODULE, (mods.get(LEAF_MODULE) ?? '').replace('const LOCAL_VALUE = 1;', 'const LOCAL_VALUE = 2;')]])); check('a head that moved nothing fails', verifyMembers(tm, input, input, mods, mods, opts).failures.some((f) => f.startsWith('nothing moved'))); + + // #30572's two shapes: a header a hand edit wrote on after, and a docblock link the base left unresolved. + const baseLife = mods.get('engine-lifecycle.ts') ?? ''; + const baseHeader = headerNamesOf(baseLife, 'Lifecycle'); + const withHeader = (lines) => baseLife.slice(0, baseHeader.region.start) + lines.join('\n') + baseLife.slice(baseHeader.region.end); + const tail = '`handWritten` was written here, beside the gate whose verdict it answers.'.split(' '); + const handMods = new Map([...mods, ['engine-lifecycle.ts', withHeader(moduleHeaderLines('Lifecycle', baseHeader.names, tail))]]); + const hand = generateMembers(tm, input, handMods, planMembers(tm, input, handMods, names, opts)); + const handHead = new Map([...handMods, ...hand.modules]); + const handLife = hand.modules.get('engine-lifecycle.ts') ?? ''; + const handRead = headerNamesOf(handLife, 'Lifecycle'); + const hv = verifyMembers(tm, input, hand.host, handMods, handHead, opts); + check('a header a hand edit wrote on after is accepted: the list grows, the hand\'s words follow it unedited, and the moves prove', + hv.failures.length === 0 && hv.moved.length === 2 && handRead?.tail.join(' ') === tail.join(' ') && + handRead.names.join() === (headerNamesOf(lifecycle, 'Lifecycle')?.names ?? []).join(), hv.failures.join(' | ')); + check('a hand tail that does not rewrap as the pass wraps it refuses', refused(() => generateMembers(tm, input, + new Map([...mods, ['engine-lifecycle.ts', withHeader([...moduleHeaderLines('Lifecycle', baseHeader.names), ' * `handWritten`', ' * was', ' * written', ' * here.'])]]), + planMembers(tm, input, mods, names, opts)))); + check('one edited character in the hand\'s words at head fails', + verifyMembers(tm, input, hand.host, handMods, new Map([...handHead, ['engine-lifecycle.ts', handLife.replace('verdict it answers.', 'verdict it answers!')]]), opts).failures.length > 0); + const linkDoc = (extra) => baseLife.replace(placeholderDoc('writes'), `/** The body of \`ObjectQL#writes\`; see {@link FourthShape}${extra}. */`); + const linkMods = new Map([...mods, ['engine-lifecycle.ts', linkDoc('')]]); + const link = generateMembers(tm, input, linkMods, planMembers(tm, input, linkMods, names, opts)); + const linkHead = new Map([...linkMods, ...link.modules]); + const linkLife = link.modules.get('engine-lifecycle.ts') ?? ''; + const lv = verifyMembers(tm, input, link.host, linkMods, linkHead, opts); + check('a base docblock link a moved body\'s import comes to resolve: the undo drops the binding the move added, and the moves prove byte for byte', + linkLife.includes("import type { FourthShape } from './fourth-shape.js';") && !linkDoc('').includes("from './fourth-shape.js'") && + lv.failures.length === 0 && lv.moved.length === 2, lv.failures.join(' | ')); + { + const extraMods = new Map([...mods, ['engine-lifecycle.ts', linkDoc(' and {@link ExtraLinked}')]]); + const extra = generateMembers(tm, input, extraMods, planMembers(tm, input, extraMods, names, opts)); + const extraLife = (extra.modules.get('engine-lifecycle.ts') ?? '').replace("import type { FourthShape } from './fourth-shape.js';", "import type { ExtraLinked, FourthShape } from './fourth-shape.js';"); + const ev = verifyMembers(tm, input, extra.host, extraMods, new Map([...extraMods, ...extra.modules, ['engine-lifecycle.ts', extraLife]]), opts); + check('a binding the head added that only a staying docblock links, and no moved member reads, fails', + extraLife.includes('ExtraLinked, FourthShape') && ev.failures.some((f) => f.includes('ExtraLinked') && f.includes('no moved member reads')), ev.failures.join(' | ')); + } } const declared = Object.keys(SELF_TEST_BATTERIES); From 0d2b37ccd8ea95e88ccf91c9fd386cb9fd753189 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 21:45:28 +0000 Subject: [PATCH 2/3] refactor(objectql): move registerApp, registerDriver and registerHook onto their facets Written by scripts/codemod-split-engine.mjs --members (the undo proof passes byte for byte); their three facade-delegation-registry exceptions are dropped, so check-facade-delegation-only holds them as delegations. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_016WV4cwSsWgu5WEWLNtFrf3 --- .../src/engine-hooks-and-functions.ts | 86 +++++- packages/objectql/src/engine-registration.ts | 206 ++++++++++++++- packages/objectql/src/engine.ts | 246 ++---------------- scripts/facade-delegation-registry.json | 21 -- 4 files changed, 297 insertions(+), 262 deletions(-) diff --git a/packages/objectql/src/engine-hooks-and-functions.ts b/packages/objectql/src/engine-hooks-and-functions.ts index 26198f35ef9..7f31d0aee40 100644 --- a/packages/objectql/src/engine-hooks-and-functions.ts +++ b/packages/objectql/src/engine-hooks-and-functions.ts @@ -4,7 +4,7 @@ * [#29986 codemod] The * "Class head: units, hooks, functions, body runners, hook dispatch" section of * `ObjectQL`: the bodies of `recordResidualOwner`, `residualOwnerSource`, - * `getStatus`, `unregisterHooksByPackage`, `registerFunction`, + * `getStatus`, `registerHook`, `unregisterHooksByPackage`, `registerFunction`, * `resolveFunction`, `resolveFunctionEntry`, `unregisterFunctionsByPackage`, * `setDefaultBodyRunner`, `getDefaultBodyRunner`, `setDefaultActionRunner`, * `getDefaultActionRunner`, `triggerHooks`, `hasHooksFor`, @@ -42,8 +42,11 @@ import { HookTargetRebindError } from './hook-target-rebind-errors.js'; import { declaredHookCondition } from './hook-wrappers.js'; import { CoreServiceName } from '@objectstack/spec/system'; import { bindHookNotice } from './hook-notice.js'; -import { hookMatchesObject } from './engine-top-level.js'; -import type { FunctionEntry, FunctionRegistrationOptions, HookHandler } from './engine-top-level.js'; +import { + DISPATCHABLE_HOOK_EVENTS, assertDispatchableHookEvent, assertHookScopeNotSelfCancelling, + assertValidHookExcludeObjects, assertValidHookObject, assertValidUnscopedMultiWriteFlag, hookMatchesObject, +} from './engine-top-level.js'; +import type { FunctionEntry, FunctionRegistrationOptions, HookEntry, HookHandler } from './engine-top-level.js'; import type { ObjectQL } from './engine.js'; /** @@ -95,6 +98,83 @@ export class HooksAndFunctionsFacet { }; } + /** The body of `ObjectQL#registerHook`; the method's docblock there is its contract. */ + static registerHook(this: Pick, event: string, handler: HookHandler, options?: { + object?: string | string[]; + /** + * [#5928] Object name(s) subtracted from what `object` admits — the way to + * say "global, except these". See {@link HookEntry.excludeObjects} for why + * an allow list cannot express it, and + * {@link assertValidHookExcludeObjects} for the two refused shapes. + */ + excludeObjects?: string | string[]; + priority?: number; + packageId?: string; + /** + * [#9719, widened by #9974] Opt-in whole-operation dispatch for an + * UNSCOPED predicate write (`multi: true`, no `where`) — see + * {@link HookEntry.dispatchUnscopedMultiWrite}. Valid on `beforeUpdate` / + * `beforeDelete` registrations only. + */ + dispatchUnscopedMultiWrite?: boolean; + /** Original metadata Hook definition (set by `bindHooksToEngine`). */ + meta?: any; + /** Stable name from metadata (set by `bindHooksToEngine`). */ + hookName?: string; + }): void { + // Refuse a scope that is statically decidable as meaningless before + // anything is registered or reported. Each face is checked on its own + // first, so the combined check below can assume well-formed names. + // [#5928] An exclusion face that subtracts nothing (`''`) or everything (`'*'`). + assertValidHookExcludeObjects(options?.excludeObjects, event); + // [commit 708431313] An allow face that names nothing (`''` → global, `[]`/`['']` → never fires). + assertValidHookObject(options?.object, event); + // [commit 708431313] Two well-formed faces that cancel out (`'account'` minus `'account'`). + assertHookScopeNotSelfCancelling(options?.object, options?.excludeObjects, event); + // [#9719/#9974] The unscoped-multi-write flag on an event whose dispatch never reads it. + assertValidUnscopedMultiWriteFlag(options?.dispatchUnscopedMultiWrite, event); + // [#17713] An event inside the engine's OWN lifecycle namespace that the + // engine never dispatches — `beforeCount`, `beforeFindOne` and their four + // siblings. #3195 warned and registered anyway; the handler then never ran, + // which on a read filter is a guardrail the author believes they armed. + // Refused at the door, with the per-seam repair named. + assertDispatchableHookEvent(event); + // [#3195] Guard against enum-vs-dispatch drift for every OTHER unknown + // name: a hook on an event the engine never triggers would register + // "successfully" and then silently never fire. Warn loudly rather than + // swallow it. Still not a hard reject here — a custom driver/plugin may + // dispatch its own events via `triggerHooks`, and outside the engine's own + // vocabulary that is the likely reading (#17713 keeps it intact). + if (!DISPATCHABLE_HOOK_EVENTS.has(event)) { + this.logger.warn( + `Hook registered for '${event}', which the engine never dispatches — it will never fire. ` + + `Dispatchable events: ${[...DISPATCHABLE_HOOK_EVENTS].join(', ')}. ` + + `(Read filtering → RLS/permissions; field masking → field metadata; delete guards → beforeDelete.)`, + { event, object: options?.object, hookName: options?.hookName }, + ); + } + if (!this.hooks.has(event)) { + this.hooks.set(event, []); + } + const entries = this.hooks.get(event)!; + entries.push({ + handler, + object: options?.object, + excludeObjects: options?.excludeObjects, + priority: options?.priority ?? 100, + packageId: options?.packageId, + dispatchUnscopedMultiWrite: options?.dispatchUnscopedMultiWrite, + meta: options?.meta, + hookName: options?.hookName, + }); + // Sort by priority (lower runs first) + entries.sort((a, b) => a.priority - b.priority); + // The exclusion face is reported alongside the allow face: a registration's + // scope is BOTH halves (#5928), and a log that printed only `object` would + // describe a global hook for an entry that is global-minus-twenty-tables. + this.logger.debug('Registered hook', { event, object: options?.object, excludeObjects: options?.excludeObjects, priority: options?.priority ?? 100, totalHandlers: entries.length }); + } + /** The body of `ObjectQL#unregisterHooksByPackage`; the method's docblock there is its contract. */ static unregisterHooksByPackage(this: Pick, packageId: string): number { if (!packageId) return 0; diff --git a/packages/objectql/src/engine-registration.ts b/packages/objectql/src/engine-registration.ts index 273a880d117..9937f9ce060 100644 --- a/packages/objectql/src/engine-registration.ts +++ b/packages/objectql/src/engine-registration.ts @@ -3,14 +3,15 @@ /** * [#29986 codemod] The * "Package, app, plugin, driver and datasource registration" section of - * `ObjectQL`: the bodies of `registerPackageBody`, - * `registerMetadataCollections`, `supplyFileColumnsMovedResolver`, - * `unregisterDriver`, `registerDatasourceDef`, `applyExternalDriverFloor`, - * `listDatasourceDefs`, `markDatasourceUnavailable`, - * `clearDatasourceUnavailable`, `getDefaultDriverName`, - * `listUnavailableDatasources`, `assertWriteAllowed` moved out of `engine.ts` - * whole by `scripts/codemod-split-engine.mjs`. `isExternalWriteForbidden` - * (#30370) was written here, beside the gate whose verdict it answers. + * `ObjectQL`: the bodies of `registerApp`, `registerPackageBody`, + * `registerMetadataCollections`, `registerDriver`, + * `supplyFileColumnsMovedResolver`, `unregisterDriver`, + * `registerDatasourceDef`, `applyExternalDriverFloor`, `listDatasourceDefs`, + * `markDatasourceUnavailable`, `clearDatasourceUnavailable`, + * `getDefaultDriverName`, `listUnavailableDatasources`, `assertWriteAllowed` + * moved out of `engine.ts` whole by `scripts/codemod-split-engine.mjs`. + * `isExternalWriteForbidden` (#30370) was written here, beside the gate whose + * verdict it answers. * * A MOVE, not a redesign. {@link RegistrationFacet} is a stateless facet of `ObjectQL` * (ruling 6624345766): each static method of it is the body of the `ObjectQL` @@ -44,7 +45,8 @@ import type { ArtifactInstallScope } from './registry.js'; import { ASSEMBLED_VIEW_ITEMS_KEY, AssembledViewArtifactSchema, expandViewContainer, isAggregatedViewContainer } from '@objectstack/spec'; import { createExternalDriverFloor, isExternalDriverFloor, needsExternalDriverFloor, unwrapExternalDriverFloor } from './external-driver-floor.js'; import { - METADATA_ARRAY_KEYS, assembledViewItemRefusal, noteRegistrationStamp, registerAppNotingStamp, + METADATA_ARRAY_KEYS, assembledViewItemRefusal, noteRegistrationStamp, packageBodyCollectionRefusal, + packageBodyObjectAndAppOwnerStampRefusal, packageBodySeedDataRefusal, registerAppNotingStamp, resolveMetadataItemName, stampObjectPackage, stampPackage, } from './engine-top-level.js'; import type { DatasourceDef } from './engine-top-level.js'; @@ -60,19 +62,157 @@ import type { ObjectQL } from './engine.js'; export interface RegistrationHost { readonly _registry: ObjectQL['_registry']; applyExternalDriverFloor: ObjectQL['applyExternalDriverFloor']; + convertPackageBodyForward: ObjectQL['convertPackageBodyForward']; readonly datasourceDefs: ObjectQL['datasourceDefs']; defaultDriver: ObjectQL['defaultDriver']; readonly drivers: ObjectQL['drivers']; haveFileColumnsMoved: ObjectQL['haveFileColumnsMoved']; + invalidateSummaryIndex: ObjectQL['invalidateSummaryIndex']; readonly logger: ObjectQL['logger']; + readonly manifests: ObjectQL['manifests']; + registerDatasourceDef: ObjectQL['registerDatasourceDef']; registerMetadataCollections: ObjectQL['registerMetadataCollections']; + registerPackageBody: ObjectQL['registerPackageBody']; registerPlugin: ObjectQL['registerPlugin']; resolveNavObjectNames: ObjectQL['resolveNavObjectNames']; + supplyFileColumnsMovedResolver: ObjectQL['supplyFileColumnsMovedResolver']; + supplyTenantKeyCertifiedResolver: ObjectQL['supplyTenantKeyCertifiedResolver']; readonly unavailableDatasources: ObjectQL['unavailableDatasources']; } /** The bodies of the "Package, app, plugin, driver and datasource registration" section of `ObjectQL`: a stateless facet, called by the class's stubs. */ export class RegistrationFacet { + /** The body of `ObjectQL#registerApp`; the method's docblock there is its contract. */ + static registerApp(this: Pick, registeredBody: any, scope?: ArtifactInstallScope, source: Exclude = 'code'): void { + // The CONVERTED body, for everything below — the package record, the + // live manifest, every item it registers (see {@link convertPackageBodyForward}). + // [#23972] Refused before anything registers — see the docblock: an + // untyped caller handing this door `'authored'` (or any other value) + // would record a row whose origin contradicts the read-only verdict + // this door's `manifests` entry produces. + if (source !== 'code' && source !== 'marketplace') { + throw new TypeError( + `[ObjectQL] registerApp: source must be 'code' or 'marketplace'; received ${JSON.stringify(source)}. ` + + `An authored package is installed through the protocol's installPackage, not registered here.`, + ); + } + const manifest = this.convertPackageBodyForward(registeredBody); + const id = manifest.id || manifest.name; + const namespace = manifest.namespace as string | undefined; + // Stamped beside `id` on every item below ({@link stampPackage}), so an + // item served from this path tells an upgrade's new item from its old one. + const version = manifest.version as string | undefined; + // [#28697] The owner-stamp refusal for the objects and apps below, which + // file through verbs of their own (steps 2-4), asked before anything of + // this body registers. See {@link packageBodyObjectAndAppOwnerStampRefusal}. + { + const refusal = packageBodyObjectAndAppOwnerStampRefusal(manifest, id, { + apps: true, + selfAsApp: Boolean(manifest.name && manifest.navigation && !manifest.apps?.length), + }); + if (refusal) throw refusal; + // …and each nested plugin's objects and plugin-as-app (step 7, which + // registers them under this package after everything above), so a + // nested plugin's stamp is refused before this body registers too. + // `registerPlugin` asks the same of the plugin it is handed. + if (Array.isArray(manifest.plugins)) { + for (const plugin of manifest.plugins) { + if (!plugin || typeof plugin !== 'object') continue; + const nested = packageBodyObjectAndAppOwnerStampRefusal(plugin, id, { + apps: false, + selfAsApp: Boolean(plugin.name && plugin.navigation), + }); + if (nested) throw nested; + } + } + // [#28697] …and every refusal the collection seam (steps 5 and 7) + // and the seed step (step 6) can judge from the body alone, so a + // body refused there leaves nothing of itself behind either, and a + // refused re-register of an installed id leaves that install as it + // was. See {@link packageBodyCollectionRefusal}. + const collections = packageBodyCollectionRefusal(manifest, id, 'manifest'); + if (collections) throw collections; + const seed = packageBodySeedDataRefusal(manifest, id); + if (seed) throw seed; + if (Array.isArray(manifest.plugins)) { + for (const plugin of manifest.plugins) { + if (!plugin || typeof plugin !== 'object') continue; + const nested = packageBodyCollectionRefusal(plugin, id, 'nested plugin'); + if (nested) throw nested; + } + } + } + this.invalidateSummaryIndex(); // new objects may add/change summary fields + this.logger.debug('Registering package manifest', { id, namespace }); + + // [#28697] What remains can only be refused part-way, by the registry's + // own verbs (an object another package owns, a field with no `type`, a + // picklist value already added, …). Refused, the registry puts back + // exactly the entries this registration wrote: a first install leaves no + // package record, object, app, item or namespace of itself, and a + // re-register of an installed id (an upgrade) leaves the installed + // version serving whole. Whatever sat there before — a tenant's object + // or overlay this package layers over (ADR-0029 D9 §6.1), another + // package's extension of its objects — reads as it did. Never through + // uninstall, which refuses an extended object and drops a tenant's + // overlay with its owner (D9.7). See {@link SchemaRegistry.restoringOnRefusal}. + this._registry.restoringOnRefusal(() => this.registerPackageBody(manifest, id, namespace, version, source, scope)); + + // [#28697] Written only once nothing above threw, so a refused body + // leaves none of them. Nothing above reads any of them. + + // Store manifest for defaultDatasource lookup + if (id) { + this.manifests.set(id, manifest); + } + + // Index datasource definitions (ADR-0015) so the write gate can read + // schemaMode + external.allowWrites. Manifests may carry `datasources` + // as an array or a name-keyed map. + if (manifest.datasources) { + const dsList = Array.isArray(manifest.datasources) + ? manifest.datasources + : Object.entries(manifest.datasources).map(([name, def]) => ({ name, ...(def as any) })); + for (const ds of dsList) { + if (ds?.name) this.registerDatasourceDef(ds); + } + } + + // Register navigation contributions (ADR-0029 D7) — nav items this + // package injects into apps owned by other packages (e.g. a capability + // plugin adding its menu into the `setup` app). Merged into the target + // app's navigation on read by group id + priority, so registration order + // does not matter. + if (Array.isArray((manifest as any).navigationContributions) && (manifest as any).navigationContributions.length > 0) { + for (const contribution of (manifest as any).navigationContributions) { + this._registry.registerAppNavContribution(contribution, id); + } + this.logger.debug('Registered navigation contributions', { + from: id, + count: (manifest as any).navigationContributions.length, + }); + } + + // Register contributed kinds (owned by no package, so written last). + if (manifest.contributes?.kinds) { + this.logger.debug('Registering kinds from manifest', { id, kindCount: manifest.contributes.kinds.length }); + for (const kind of manifest.contributes.kinds) { + this._registry.registerKind(kind); + // [commit 10485009a] Name the kind by its declared `id`. `contributes.kinds` + // items are `{ id, description? }` (`manifest.zod.ts`; `globs` was + // retired unread, #11169) and + // `registerKind` keys the item on `id` (`registerItem('kind', kind, 'id')`), + // so `id` is BOTH the only identifying field the schema declares and the + // exact key the item is stored under — a reader of this line can look the + // item straight back up. The previous `kind.name || kind.type` reached for + // two fields NEITHER shape declares, so every conforming manifest logged + // `kind: undefined`. Do not re-add those as a fallback: reading undeclared + // aliases here is the consumer-side tolerance Prime Directive #12 rejects. + this.logger.debug('Registered Kind', { kind: kind.id, from: id }); + } + } + } + /** * [#28697] The part of {@link registerApp} a registry verb can refuse * part-way: the package record, its objects, extensions, apps, collections, @@ -331,6 +471,54 @@ export class RegistrationFacet { } } + /** The body of `ObjectQL#registerDriver`; the method's docblock there is its contract. */ + static registerDriver(this: Pick, driver: IDataDriver, isDefault: boolean = false): void { + const existing = this.drivers.get(driver.name); + if (existing) { + // [#27277] The entry may be the floored instance of this very driver. + if (unwrapExternalDriverFloor(existing) !== unwrapExternalDriverFloor(driver)) { + this.logger.warn( + 'Driver name collision — KEEPING the already-registered driver and DISCARDING the one just supplied. ' + + 'Two different driver instances claim one name, so whatever configuration the discarded instance carried ' + + '(connection string, pool, capabilities) is NOT in force, while queries routed to this name keep working ' + + 'against the one that was kept. Fix the caller: give the second datasource a name of its own.', + { + driverName: driver.name, + keptVersion: existing.version, + discardedVersion: driver.version, + }, + ); + } else if (isDefault && this.defaultDriver !== driver.name) { + this.logger.warn( + 'Driver re-registered as DEFAULT but another driver already holds that role — the request is IGNORED and ' + + 'the existing default stands. Unregister the current default first if the switch was intended.', + { driverName: driver.name, currentDefault: this.defaultDriver }, + ); + } else { + // The by-design round trip documented above — expected on every boot, + // so it must not reach the boot-diagnostics warning list. + this.logger.debug('Driver already registered — re-registering the same instance is a no-op', { + driverName: driver.name, + }); + } + return; + } + + this.drivers.set(driver.name, driver); + this.supplyFileColumnsMovedResolver(driver); + this.supplyTenantKeyCertifiedResolver(driver); + this.applyExternalDriverFloor(driver.name); + this.logger.info('Registered driver', { + driverName: driver.name, + version: driver.version + }); + + if (isDefault || this.drivers.size === 1) { + this.defaultDriver = driver.name; + this.logger.info('Set default driver', { driverName: driver.name }); + } + } + /** * Hand a freshly-registered driver the ADR-0104 media arm — the kernel→driver * supply seam (#15989, sequencing step 2 of ADR-0104's 2026-09-05 addendum). diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 8f683cab7d8..c2216269cc6 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -304,7 +304,6 @@ import { import { externalPredicateWriteRefusal, isExternalDriverFloor, - unwrapExternalDriverFloor, } from './external-driver-floor.js'; // [#25522 cut 14] The master-detail landing, the preview's parents and the // related reads a validation rule makes; the methods of the same names below @@ -359,13 +358,11 @@ import { DISPATCHABLE_HOOK_EVENTS, ENGINE_DELETE_OPTION_KEYS, ENGINE_FIND_OPTION_KEYS, ENGINE_QUERY_SLOTS, ENGINE_UPDATE_OPTION_KEYS, ENGINE_WHERE_SLOTS, ENGINE_WIRE_ONLY_SLOTS, SECURITY_SERVICE_SOURCE_SLOT, WRITE_PRECONDITION_OPTION, applyFormulaPlan, applyStoredExpressionsToPatch, assertDispatchableHookEvent, - assertHookScopeNotSelfCancelling, assertOrderByIsMaterializable, assertProjectionHasNoDottedPaths, - assertValidHookExcludeObjects, assertValidHookObject, assertValidUnscopedMultiWriteFlag, autonumberFieldNames, - callerKeySets, callerSuppliedRow, computeStoredExpressions, declaredDatetimeLowering, droppedFieldEvents, - foldEngineOptionAliases, formulaPlanEntry, handBackWithheldInsertReadonly, hydrateWriteFormulas, + assertOrderByIsMaterializable, assertProjectionHasNoDottedPaths, assertValidHookExcludeObjects, + autonumberFieldNames, callerKeySets, callerSuppliedRow, computeStoredExpressions, declaredDatetimeLowering, + droppedFieldEvents, foldEngineOptionAliases, formulaPlanEntry, handBackWithheldInsertReadonly, hydrateWriteFormulas, insertStaticReadonlySubject, lowerWhereFilterArray, masterIdOf, mergeReadContext, - organizationPredicateDeleteRefusal, packageBodyCollectionRefusal, packageBodyObjectAndAppOwnerStampRefusal, - packageBodySeedDataRefusal, planFormulaProjection, planStoredExpressions, recomputeAutonumberReaders, + organizationPredicateDeleteRefusal, planFormulaProjection, planStoredExpressions, recomputeAutonumberReaders, rejectUnknownEngineOptions, replacedStoredExpressionFields, rowsWithoutFormulaWidening, settleBulkStoredExpressions, shouldSkipStateMachine, staticReadonlyStoredView, storedExpressionsReading, stripComputedWriteFields, undeclaredWriteFieldErrors, updateStaticReadonlySubject, withFileOwnerKey, withholdInsertReadonlyFromHooks, @@ -812,57 +809,8 @@ export class ObjectQL implements IObjectQLEngine { /** Stable name from metadata (set by `bindHooksToEngine`). */ hookName?: string; }): void { - // Refuse a scope that is statically decidable as meaningless before - // anything is registered or reported. Each face is checked on its own - // first, so the combined check below can assume well-formed names. - // [#5928] An exclusion face that subtracts nothing (`''`) or everything (`'*'`). - assertValidHookExcludeObjects(options?.excludeObjects, event); - // [commit 708431313] An allow face that names nothing (`''` → global, `[]`/`['']` → never fires). - assertValidHookObject(options?.object, event); - // [commit 708431313] Two well-formed faces that cancel out (`'account'` minus `'account'`). - assertHookScopeNotSelfCancelling(options?.object, options?.excludeObjects, event); - // [#9719/#9974] The unscoped-multi-write flag on an event whose dispatch never reads it. - assertValidUnscopedMultiWriteFlag(options?.dispatchUnscopedMultiWrite, event); - // [#17713] An event inside the engine's OWN lifecycle namespace that the - // engine never dispatches — `beforeCount`, `beforeFindOne` and their four - // siblings. #3195 warned and registered anyway; the handler then never ran, - // which on a read filter is a guardrail the author believes they armed. - // Refused at the door, with the per-seam repair named. - assertDispatchableHookEvent(event); - // [#3195] Guard against enum-vs-dispatch drift for every OTHER unknown - // name: a hook on an event the engine never triggers would register - // "successfully" and then silently never fire. Warn loudly rather than - // swallow it. Still not a hard reject here — a custom driver/plugin may - // dispatch its own events via `triggerHooks`, and outside the engine's own - // vocabulary that is the likely reading (#17713 keeps it intact). - if (!DISPATCHABLE_HOOK_EVENTS.has(event)) { - this.logger.warn( - `Hook registered for '${event}', which the engine never dispatches — it will never fire. ` + - `Dispatchable events: ${[...DISPATCHABLE_HOOK_EVENTS].join(', ')}. ` + - `(Read filtering → RLS/permissions; field masking → field metadata; delete guards → beforeDelete.)`, - { event, object: options?.object, hookName: options?.hookName }, - ); - } - if (!this.hooks.has(event)) { - this.hooks.set(event, []); - } - const entries = this.hooks.get(event)!; - entries.push({ - handler, - object: options?.object, - excludeObjects: options?.excludeObjects, - priority: options?.priority ?? 100, - packageId: options?.packageId, - dispatchUnscopedMultiWrite: options?.dispatchUnscopedMultiWrite, - meta: options?.meta, - hookName: options?.hookName, - }); - // Sort by priority (lower runs first) - entries.sort((a, b) => a.priority - b.priority); - // The exclusion face is reported alongside the allow face: a registration's - // scope is BOTH halves (#5928), and a log that printed only `object` would - // describe a global hook for an entry that is global-minus-twenty-tables. - this.logger.debug('Registered hook', { event, object: options?.object, excludeObjects: options?.excludeObjects, priority: options?.priority ?? 100, totalHandlers: entries.length }); + // [#29986 codemod] The body moved to `engine-hooks-and-functions.ts` (see `HooksAndFunctionsHost`). + return HooksAndFunctionsFacet.registerHook.call(hooksAndFunctionsHost.of(this), event, handler, options); } /** @@ -2308,133 +2256,8 @@ export class ObjectQL implements IObjectQLEngine { * `@objectstack/metadata-protocol`), so `'authored'` is not accepted. */ registerApp(registeredBody: any, scope?: ArtifactInstallScope, source: Exclude = 'code'): void { - // The CONVERTED body, for everything below — the package record, the - // live manifest, every item it registers (see {@link convertPackageBodyForward}). - // [#23972] Refused before anything registers — see the docblock: an - // untyped caller handing this door `'authored'` (or any other value) - // would record a row whose origin contradicts the read-only verdict - // this door's `manifests` entry produces. - if (source !== 'code' && source !== 'marketplace') { - throw new TypeError( - `[ObjectQL] registerApp: source must be 'code' or 'marketplace'; received ${JSON.stringify(source)}. ` - + `An authored package is installed through the protocol's installPackage, not registered here.`, - ); - } - const manifest = this.convertPackageBodyForward(registeredBody); - const id = manifest.id || manifest.name; - const namespace = manifest.namespace as string | undefined; - // Stamped beside `id` on every item below ({@link stampPackage}), so an - // item served from this path tells an upgrade's new item from its old one. - const version = manifest.version as string | undefined; - // [#28697] The owner-stamp refusal for the objects and apps below, which - // file through verbs of their own (steps 2-4), asked before anything of - // this body registers. See {@link packageBodyObjectAndAppOwnerStampRefusal}. - { - const refusal = packageBodyObjectAndAppOwnerStampRefusal(manifest, id, { - apps: true, - selfAsApp: Boolean(manifest.name && manifest.navigation && !manifest.apps?.length), - }); - if (refusal) throw refusal; - // …and each nested plugin's objects and plugin-as-app (step 7, which - // registers them under this package after everything above), so a - // nested plugin's stamp is refused before this body registers too. - // `registerPlugin` asks the same of the plugin it is handed. - if (Array.isArray(manifest.plugins)) { - for (const plugin of manifest.plugins) { - if (!plugin || typeof plugin !== 'object') continue; - const nested = packageBodyObjectAndAppOwnerStampRefusal(plugin, id, { - apps: false, - selfAsApp: Boolean(plugin.name && plugin.navigation), - }); - if (nested) throw nested; - } - } - // [#28697] …and every refusal the collection seam (steps 5 and 7) - // and the seed step (step 6) can judge from the body alone, so a - // body refused there leaves nothing of itself behind either, and a - // refused re-register of an installed id leaves that install as it - // was. See {@link packageBodyCollectionRefusal}. - const collections = packageBodyCollectionRefusal(manifest, id, 'manifest'); - if (collections) throw collections; - const seed = packageBodySeedDataRefusal(manifest, id); - if (seed) throw seed; - if (Array.isArray(manifest.plugins)) { - for (const plugin of manifest.plugins) { - if (!plugin || typeof plugin !== 'object') continue; - const nested = packageBodyCollectionRefusal(plugin, id, 'nested plugin'); - if (nested) throw nested; - } - } - } - this.invalidateSummaryIndex(); // new objects may add/change summary fields - this.logger.debug('Registering package manifest', { id, namespace }); - - // [#28697] What remains can only be refused part-way, by the registry's - // own verbs (an object another package owns, a field with no `type`, a - // picklist value already added, …). Refused, the registry puts back - // exactly the entries this registration wrote: a first install leaves no - // package record, object, app, item or namespace of itself, and a - // re-register of an installed id (an upgrade) leaves the installed - // version serving whole. Whatever sat there before — a tenant's object - // or overlay this package layers over (ADR-0029 D9 §6.1), another - // package's extension of its objects — reads as it did. Never through - // uninstall, which refuses an extended object and drops a tenant's - // overlay with its owner (D9.7). See {@link SchemaRegistry.restoringOnRefusal}. - this._registry.restoringOnRefusal(() => this.registerPackageBody(manifest, id, namespace, version, source, scope)); - - // [#28697] Written only once nothing above threw, so a refused body - // leaves none of them. Nothing above reads any of them. - - // Store manifest for defaultDatasource lookup - if (id) { - this.manifests.set(id, manifest); - } - - // Index datasource definitions (ADR-0015) so the write gate can read - // schemaMode + external.allowWrites. Manifests may carry `datasources` - // as an array or a name-keyed map. - if (manifest.datasources) { - const dsList = Array.isArray(manifest.datasources) - ? manifest.datasources - : Object.entries(manifest.datasources).map(([name, def]) => ({ name, ...(def as any) })); - for (const ds of dsList) { - if (ds?.name) this.registerDatasourceDef(ds); - } - } - - // Register navigation contributions (ADR-0029 D7) — nav items this - // package injects into apps owned by other packages (e.g. a capability - // plugin adding its menu into the `setup` app). Merged into the target - // app's navigation on read by group id + priority, so registration order - // does not matter. - if (Array.isArray((manifest as any).navigationContributions) && (manifest as any).navigationContributions.length > 0) { - for (const contribution of (manifest as any).navigationContributions) { - this._registry.registerAppNavContribution(contribution, id); - } - this.logger.debug('Registered navigation contributions', { - from: id, - count: (manifest as any).navigationContributions.length, - }); - } - - // Register contributed kinds (owned by no package, so written last). - if (manifest.contributes?.kinds) { - this.logger.debug('Registering kinds from manifest', { id, kindCount: manifest.contributes.kinds.length }); - for (const kind of manifest.contributes.kinds) { - this._registry.registerKind(kind); - // [commit 10485009a] Name the kind by its declared `id`. `contributes.kinds` - // items are `{ id, description? }` (`manifest.zod.ts`; `globs` was - // retired unread, #11169) and - // `registerKind` keys the item on `id` (`registerItem('kind', kind, 'id')`), - // so `id` is BOTH the only identifying field the schema declares and the - // exact key the item is stored under — a reader of this line can look the - // item straight back up. The previous `kind.name || kind.type` reached for - // two fields NEITHER shape declares, so every conforming manifest logged - // `kind: undefined`. Do not re-add those as a fallback: reading undeclared - // aliases here is the consumer-side tolerance Prime Directive #12 rejects. - this.logger.debug('Registered Kind', { kind: kind.id, from: id }); - } - } + // [#29986 codemod] The body moved to `engine-registration.ts` (see `RegistrationHost`). + return RegistrationFacet.registerApp.call(registrationHost.of(this), registeredBody, scope, source); } private registerPackageBody( @@ -2509,50 +2332,8 @@ export class ObjectQL implements IObjectQLEngine { * so it is loud too rather than folded into the quiet path. */ registerDriver(driver: IDataDriver, isDefault: boolean = false): void { - const existing = this.drivers.get(driver.name); - if (existing) { - // [#27277] The entry may be the floored instance of this very driver. - if (unwrapExternalDriverFloor(existing) !== unwrapExternalDriverFloor(driver)) { - this.logger.warn( - 'Driver name collision — KEEPING the already-registered driver and DISCARDING the one just supplied. ' + - 'Two different driver instances claim one name, so whatever configuration the discarded instance carried ' + - '(connection string, pool, capabilities) is NOT in force, while queries routed to this name keep working ' + - 'against the one that was kept. Fix the caller: give the second datasource a name of its own.', - { - driverName: driver.name, - keptVersion: existing.version, - discardedVersion: driver.version, - }, - ); - } else if (isDefault && this.defaultDriver !== driver.name) { - this.logger.warn( - 'Driver re-registered as DEFAULT but another driver already holds that role — the request is IGNORED and ' + - 'the existing default stands. Unregister the current default first if the switch was intended.', - { driverName: driver.name, currentDefault: this.defaultDriver }, - ); - } else { - // The by-design round trip documented above — expected on every boot, - // so it must not reach the boot-diagnostics warning list. - this.logger.debug('Driver already registered — re-registering the same instance is a no-op', { - driverName: driver.name, - }); - } - return; - } - - this.drivers.set(driver.name, driver); - this.supplyFileColumnsMovedResolver(driver); - this.supplyTenantKeyCertifiedResolver(driver); - this.applyExternalDriverFloor(driver.name); - this.logger.info('Registered driver', { - driverName: driver.name, - version: driver.version - }); - - if (isDefault || this.drivers.size === 1) { - this.defaultDriver = driver.name; - this.logger.info('Set default driver', { driverName: driver.name }); - } + // [#29986 codemod] The body moved to `engine-registration.ts` (see `RegistrationHost`). + return RegistrationFacet.registerDriver.call(registrationHost.of(this), driver, isDefault); } private supplyFileColumnsMovedResolver(driver: IDataDriver): void { @@ -10560,15 +10341,22 @@ export class ObjectQL implements IObjectQLEngine { registrationHost.install((engine) => ({ get _registry() { return engine._registry; }, applyExternalDriverFloor: (...a) => engine.applyExternalDriverFloor(...a), + convertPackageBodyForward: (...a) => engine.convertPackageBodyForward(...a), get datasourceDefs() { return engine.datasourceDefs; }, get defaultDriver() { return engine.defaultDriver; }, set defaultDriver(v) { engine.defaultDriver = v; }, get drivers() { return engine.drivers; }, haveFileColumnsMoved: (...a) => engine.haveFileColumnsMoved(...a), + invalidateSummaryIndex: (...a) => engine.invalidateSummaryIndex(...a), get logger() { return engine.logger; }, + get manifests() { return engine.manifests; }, + registerDatasourceDef: (...a) => engine.registerDatasourceDef(...a), registerMetadataCollections: (...a) => engine.registerMetadataCollections(...a), + registerPackageBody: (...a) => engine.registerPackageBody(...a), registerPlugin: (...a) => engine.registerPlugin(...a), resolveNavObjectNames: (...a) => engine.resolveNavObjectNames(...a), + supplyFileColumnsMovedResolver: (...a) => engine.supplyFileColumnsMovedResolver(...a), + supplyTenantKeyCertifiedResolver: (...a) => engine.supplyTenantKeyCertifiedResolver(...a), get unavailableDatasources() { return engine.unavailableDatasources; }, })); secretsAndRoutingHost.install((engine) => ({ diff --git a/scripts/facade-delegation-registry.json b/scripts/facade-delegation-registry.json index 22b1dd10d74..1b3d43c52d8 100644 --- a/scripts/facade-delegation-registry.json +++ b/scripts/facade-delegation-registry.json @@ -291,27 +291,6 @@ "card": "#30020", "reason": "state-owning: bare-this, the body hands the instance itself on (#29986 report: stays on the class)" }, - { - "file": "packages/objectql/src/engine.ts", - "class": "ObjectQL", - "member": "registerApp", - "card": "#30054", - "reason": "return type spelled on the signature (#30054, public .d.ts byte-equal); the codemod --members pass refuses to move it: engine-registration.ts's header is no longer as the first pass wrote it (a hand-written member was added there)" - }, - { - "file": "packages/objectql/src/engine.ts", - "class": "ObjectQL", - "member": "registerHook", - "card": "#30054", - "reason": "return type spelled on the signature (#30054, public .d.ts byte-equal); the codemod --members pass cannot prove its move: undoing it does not rebuild engine-hooks-and-functions.ts byte for byte (first difference at line 44, in its import block)" - }, - { - "file": "packages/objectql/src/engine.ts", - "class": "ObjectQL", - "member": "registerDriver", - "card": "#30054", - "reason": "return type spelled on the signature (#30054, public .d.ts byte-equal); the codemod --members pass refuses to move it: engine-registration.ts's header is no longer as the first pass wrote it (a hand-written member was added there)" - }, { "file": "packages/objectql/src/engine.ts", "class": "ObjectQL", From 08d004714c9180c89fa67128c0106719ebb6b04e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 21:52:20 +0000 Subject: [PATCH 3/3] chore(changeset): objectql registration bodies move into their facets Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_016WV4cwSsWgu5WEWLNtFrf3 --- .changeset/30572-objectql-registration-bodies.md | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 .changeset/30572-objectql-registration-bodies.md diff --git a/.changeset/30572-objectql-registration-bodies.md b/.changeset/30572-objectql-registration-bodies.md new file mode 100644 index 00000000000..9d48df08380 --- /dev/null +++ b/.changeset/30572-objectql-registration-bodies.md @@ -0,0 +1,10 @@ +--- +'@objectstack/objectql': patch +--- + +refactor(objectql): the bodies of `registerApp`, `registerDriver` and `registerHook` move into their facets + +Clause-②: no +Metadata-protocol: none + +- The bodies of `ObjectQL#registerApp` and `ObjectQL#registerDriver` move out of `engine.ts` into `engine-registration.ts`, and the body of `ObjectQL#registerHook` into `engine-hooks-and-functions.ts`; the methods stay on the class as one-call stubs with their signatures and docblocks. The built declaration files are byte-equal before and after, and no behaviour changes; only the source maps move. Nothing to do on upgrade.