From 061901a07716a6f9db2c88fe3b2442efc8fa8196 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 23:03:51 +0000 Subject: [PATCH] fix(objectql): log a contributed kind by its declared `id` (#10729) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `registerApp()` logged `kind.name || kind.type` for every entry in a manifest's `contributes.kinds`. Neither field is declared: the schema says `{ id, globs, description? }` and `registerKind` types its parameter `{ id: string, globs: string[] }`, so the expression evaluated `undefined || undefined` and every conforming manifest logged `kind: undefined`. Log `kind.id` instead — the only identifying field the schema declares, and the exact key `registerItem('kind', kind, 'id')` files the descriptor under, so the log line and `registry.listItems('kind')` name the item the same way. No `?? kind.name` fallback: reading an undeclared alias in a consumer is the tolerance Prime Directive #12 rejects. Pinned by `engine-kind-registration-log.test.ts` — a debug field that silently goes `undefined` survives forever precisely because nothing asserts on it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y --- .../kind-registration-log-declared-id.md | 31 +++++ .../src/engine-kind-registration-log.test.ts | 108 ++++++++++++++++++ packages/objectql/src/engine.ts | 11 +- 3 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 .changeset/kind-registration-log-declared-id.md create mode 100644 packages/objectql/src/engine-kind-registration-log.test.ts diff --git a/.changeset/kind-registration-log-declared-id.md b/.changeset/kind-registration-log-declared-id.md new file mode 100644 index 0000000000..7f1fb2a802 --- /dev/null +++ b/.changeset/kind-registration-log-declared-id.md @@ -0,0 +1,31 @@ +--- +"@objectstack/objectql": patch +--- + +Log a contributed metadata kind by its declared `id` (#10729). `registerApp()` +emits one `'Registered Kind'` debug line per entry in a manifest's +`contributes.kinds`, and that line read `kind.name || kind.type`: + +```ts +this.logger.debug('Registered Kind', { kind: kind.name || kind.type, from: id }); +``` + +`contributes.kinds` items declare neither field. The schema +(`packages/spec/src/kernel/manifest.zod.ts`) says `{ id, globs, description? }`, +and `SchemaRegistry.registerKind` types its parameter `{ id: string, globs: string[] }` +— so the expression evaluated `undefined || undefined` and every conforming +manifest logged `kind: undefined`. The line now reads `kind.id`. + +`id` rather than any other declared field because `registerKind` files the +descriptor with `registerItem('kind', kind, 'id')`: `id` is simultaneously the +only identifying field the schema declares and the exact key the item is stored +under, so a reader of the log line can look the item back up with it. Kept +undeclared aliases OUT rather than adding `?? kind.name` for old manifests — +reading an undeclared alias in a consumer is the tolerance Prime Directive #12 +rejects, and no manifest in this repo authors the older shape. + +Behaviour change is confined to the text of one `debug`-level line; nothing +branches on it. It is pinned by `engine-kind-registration-log.test.ts`, which +asserts the logged value equals the key `registry.listItems('kind')` files the +descriptor under — a debug field that silently goes `undefined` is exactly the +class of defect that survives forever because nothing asserts on it. diff --git a/packages/objectql/src/engine-kind-registration-log.test.ts b/packages/objectql/src/engine-kind-registration-log.test.ts new file mode 100644 index 0000000000..58cc433c13 --- /dev/null +++ b/packages/objectql/src/engine-kind-registration-log.test.ts @@ -0,0 +1,108 @@ +/** + * [#10729] `contributes.kinds` — the registration site's debug line must name + * fields that EXIST. + * + * `registerApp()` logs one `'Registered Kind'` line per contributed kind. It + * used to read `kind.name || kind.type`, and `contributes.kinds` items declare + * neither: the schema (`packages/spec/src/kernel/manifest.zod.ts`) says + * `{ id, globs, description? }` and `SchemaRegistry.registerKind` types its + * parameter `{ id: string, globs: string[] }`. So the line evaluated + * `undefined || undefined` and logged `kind: undefined` for every conforming + * manifest — a defect with no failing test anywhere, because a debug field + * that silently goes `undefined` is invisible to everything except a human + * reading the log at the moment it matters. + * + * That is the whole reason this file exists. The fix is two tokens wide; the + * pin is the part that keeps it fixed. + * + * Why `id` and not something else: `registerKind` stores the descriptor with + * `registerItem('kind', kind, 'id')`, so `id` is simultaneously (a) the only + * identifying field the schema declares and (b) the exact key the item is + * filed under — which makes the log line and the registry answer the same + * question the same way. The second test pins the direction as well as the + * value: an off-spec manifest that DOES carry `name`/`type` must still be + * logged by `id`, because reading an undeclared alias in a consumer is the + * tolerance Prime Directive #12 rejects. + * + * Real engine, real `SchemaRegistry` — no doubles. The assertion is about + * what the registration seam actually does with a manifest, so a mocked + * registry would be asserting on the mock. + */ +import { describe, it, expect } from 'vitest'; +import { ObjectQL } from './engine'; + +interface DebugLine { msg: string; meta: Record | undefined } + +function engineWithRecordedDebug(): { engine: ObjectQL; lines: DebugLine[] } { + const lines: DebugLine[] = []; + const logger = { + debug: (msg: string, meta?: Record) => { lines.push({ msg, meta }); }, + info() {}, warn() {}, error() {}, + }; + return { engine: new ObjectQL({ logger } as any), lines }; +} + +const registeredKindLines = (lines: DebugLine[]): DebugLine[] => + lines.filter((l) => l.msg === 'Registered Kind'); + +describe('[#10729] contributes.kinds registration logging', () => { + it('names a conforming kind by its declared `id`, not by undeclared fields', () => { + const { engine, lines } = engineWithRecordedDebug(); + + engine.registerApp({ + id: 'com.example.bi', + contributes: { + // Exactly the schema's shape — and exactly its own documented example + // ("Registering a BI plugin to handle *.report.ts"). + kinds: [{ id: 'sys.bi.report', globs: ['**/*.report.ts'] }], + }, + }); + + const logged = registeredKindLines(lines); + expect(logged).toHaveLength(1); + expect(logged[0]!.meta).toEqual({ kind: 'sys.bi.report', from: 'com.example.bi' }); + + // The regression this pins is specifically `undefined`, so say so: a + // future edit that reintroduces an undeclared read fails HERE with a + // readable message rather than at the deep-equal above. + expect(logged[0]!.meta!.kind).toBeDefined(); + }); + + it('logs the same key the registry files the descriptor under', () => { + const { engine, lines } = engineWithRecordedDebug(); + + engine.registerApp({ + id: 'com.example.bi', + contributes: { kinds: [{ id: 'sys.bi.report', globs: ['**/*.report.ts'] }] }, + }); + + // `registerKind` → `registerItem('kind', kind, 'id')`. The value in the log + // is only useful if it is the value you can look the item back up by, so + // assert the round trip rather than the string twice. + const stored = engine.registry.listItems<{ id: string }>('kind'); + expect(stored.map((k) => k.id)).toContain(registeredKindLines(lines)[0]!.meta!.kind); + }); + + it('still logs `id` when an off-spec manifest carries `name`/`type`', () => { + const { engine, lines } = engineWithRecordedDebug(); + + engine.registerApp({ + id: 'com.legacy.bi', + contributes: { + kinds: [{ + id: 'sys.bi.report', + globs: ['**/*.report.ts'], + // Neither key is declared by the schema. They are what the old line + // reached for, so an author who copied an ancient example could put + // them here — and the log must NOT start preferring them again. + name: 'Report (undeclared)', + type: 'report (undeclared)', + }], + }, + }); + + const logged = registeredKindLines(lines); + expect(logged).toHaveLength(1); + expect(logged[0]!.meta).toEqual({ kind: 'sys.bi.report', from: 'com.legacy.bi' }); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 782ebf7414..ebb9c13e2a 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -4500,7 +4500,16 @@ export class ObjectQL implements IObjectQLEngine { this.logger.debug('Registering kinds from manifest', { id, kindCount: manifest.contributes.kinds.length }); for (const kind of manifest.contributes.kinds) { this._registry.registerKind(kind); - this.logger.debug('Registered Kind', { kind: kind.name || kind.type, from: id }); + // [#10729] Name the kind by its declared `id`. `contributes.kinds` + // items are `{ id, globs, description? }` (`manifest.zod.ts`) 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 }); } }