Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions .changeset/kind-registration-log-declared-id.md
Original file line numberDiff line numberDiff line change
@@ -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.
108 changes: 108 additions & 0 deletions packages/objectql/src/engine-kind-registration-log.test.ts
Original file line numberDiff line numberDiff line change
@@ -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<string, unknown> | undefined }

function engineWithRecordedDebug(): { engine: ObjectQL; lines: DebugLine[] } {
const lines: DebugLine[] = [];
const logger = {
debug: (msg: string, meta?: Record<string, unknown>) => { 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' });
});
});
11 changes: 10 additions & 1 deletion packages/objectql/src/engine.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 });
}
}

Expand Down
Loading