Skip to content

Commit 43d90ca

Browse files
dmealingclaude
andcommitted
fix(#194): resolve non-object references + make dbImport/dialect optional (replaces docs)
A Fable adjudication of the three #194 adopter constraints ruled two of them real bugs the prior commit had merely DOCUMENTED rather than fixed. This fixes both in code and keeps only the genuinely correct-by-design one as docs: - Item 2 (FIX): a ReferenceDescriptor's targetType is a free string, but the loader symbol table indexed only object.* nodes, so a descriptor targeting a custom top-level type resolved cleanly then unconditionally failed with a false 'does not resolve to an object'. The symbol table is now keyed per node type; a reference to any registered top-level kind resolves under the ADR-0042 contract, and a top-level non-object node establishes package context for its subtree. Object-target refs are byte-identical (2252 metadata tests green). - Item 3 (FIX): dbImport/dialect were required config even for a value-object-only project that emits zero DB code. They are now optional on the user config; the runner fills inert defaults when absent AND no DB object is present, and throws a clear error naming the entities when a DB-emitting model omits them (never a silent sqlite default for a forgotten Postgres dialect). Resolved config the generators consume stays required — DB-project output unchanged. - Item 1 (DOCUMENT): requiring registry.extend on metadata.root to license a custom top-level type is chartered fail-closed design (FR-033) — kept as docs, rewritten to a 'defining a custom top-level node type' how-to that also notes item 2's fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TJRi8FtEW24z9HKGUL1xh8
1 parent 03aacf4 commit 43d90ca

10 files changed

Lines changed: 335 additions & 65 deletions

File tree

CHANGELOG.md

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,33 @@ this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
77

88
## [Unreleased]
99

10-
**npm-only**`codegen-ts`; PyPI / NuGet / Maven Central unchanged.
10+
**npm-only**`metadata` + `codegen-ts`; PyPI / NuGet / Maven Central unchanged.
11+
12+
### Fixed — a reference to a non-`object` top-level node now resolves (#194)
13+
14+
A `ReferenceDescriptor`'s `targetType` is a free string (the mechanism promises a
15+
downstream provider's references validate "present and future"), but the loader's symbol
16+
table indexed only `object.*` nodes — so a descriptor targeting a custom top-level type
17+
(`targetType: "adapter"`) type-checked, registered, and then *unconditionally* failed
18+
every reference with a false "does not resolve to an object". The symbol table is now
19+
keyed per node type, so a reference to any registered top-level node kind resolves under
20+
the same ADR-0042 package-local contract (FQN-exact, else the referrer's package, else
21+
root-level), and the unresolved-error message names the actual target kind. Object-target
22+
references (every core `@objectRef` / `@from` / `@references` / `@payloadRef`) are
23+
byte-identical — the whole conformance corpus is unchanged; the change is strictly
24+
enabling.
25+
26+
### Fixed — `dbImport` / `dialect` are optional for a value-object-only project (#194)
27+
28+
A model that declares only `object.value`s generates zero database / query / route code,
29+
yet `dbImport` and `dialect` were **required** codegen-config fields, so a
30+
value-object-only project had to supply dead-but-mandatory placeholders to satisfy
31+
`tsc`. They are now optional on the user config; `meta gen` fills inert defaults when they
32+
are absent AND the model emits no DB artifacts, and throws a clear error naming the
33+
offending entities when they are absent but the model *does* generate DB code (so a
34+
Postgres project that forgets `dialect` gets an error, never silently-emitted sqlite).
35+
The resolved config the generators consume still carries both as required — generated
36+
output for every DB-generating project is unchanged.
1137

1238
### Added — `meta gen` records the codegen engine version and flags a change since the last run (#232)
1339

docs/features/extending-with-providers.md

Lines changed: 32 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -343,33 +343,38 @@ configuration rich.
343343
All five ports compose providers in **dependency order** (Kahn's algorithm)
344344
and emit the same stable error codes when composition fails.
345345

346-
## Adopter constraints (worth knowing up front)
347-
348-
Three constraints that surface when building a real consumer — each is resolvable,
349-
but none is obvious from the happy-path docs:
350-
351-
1. **Adding a new top-level node type requires `registry.extend` on `metadata.root`.**
352-
`metadata.root`'s child rules are closed: a custom top-level subtype (e.g. an
353-
`adapter.*` / `probe.*` you register via a provider) fails to load with
354-
`ERR_CHILD_NOT_ALLOWED` unless a provider **also** `extend`s `metadata.root`'s
355-
child rules to license it as a permitted child. Registering the subtype is not
356-
enough on its own — its container must be told the subtype is allowed there.
357-
358-
2. **Declarative reference resolution only indexes `object.*` nodes.** The built-in
359-
ref resolver (the one behind `@objectRef` / `@from` / `@references` / `@payloadRef`)
360-
binds references to `object.*` targets only. A reference **to a non-`object.*`
361-
node** (e.g. a custom `adapter.*` referencing another custom top-level node) is not
362-
resolved by the built-in resolver — resolve it yourself in the provider's `validate`
363-
hook by walking the tree. Model references to entities/values/projections and you
364-
get resolution for free; references to other node kinds are yours to resolve.
365-
366-
3. **`dbImport` / `dialect` are required config even for a value-object-only project.**
367-
A model that declares only `object.value`s (no write-through entities) generates
368-
zero database / query / route code — yet `dbImport` and `dialect` are **required**
369-
fields of the codegen config, so omitting them is a `tsc` type error. Supply inert
370-
placeholders (e.g. `dialect: "sqlite"`, `dbImport: "./db"`); they are never read
371-
when no DB code is generated. This is required regardless and harmless for
372-
value-object-only models.
346+
## Defining a custom top-level node type
347+
348+
`metadata.root`'s child rules are a **closed set** — by design (FR-033 fail-closed
349+
hardening): a document root admits `object` / `field` / `validator` / `template` nodes
350+
and nothing else. So registering a new top-level subtype via a provider is **two steps**,
351+
both in the same provider's `registerTypes`:
352+
353+
1. `registry.register(...)` the new `(type, subType)` — this declares the vocabulary
354+
*exists*, not where it may *live*.
355+
2. `registry.extend(TYPE_METADATA, SUBTYPE_ROOT, { childRules: [...] })` to **license**
356+
it as a permitted child of `metadata.root`.
357+
358+
Skip step 2 and the node fails to load with `ERR_CHILD_NOT_ALLOWED`. This is
359+
deliberate, not a papercut: registration declaring existence and the parent declaring
360+
admission are separate concerns (most registered subtypes — `view.image`,
361+
`template.toolcall` — are emphatically *not* root-level), and root admission is part of
362+
the byte-matched cross-port registry manifest. Auto-admitting every new type at the
363+
document root would be fail-open and would take admission out of the declarative record.
364+
365+
```ts
366+
registerTypes(registry) {
367+
registry.register({ typeId: new TypeId("adapter", "http"), /* … */ });
368+
registry.extend(TYPE_METADATA, SUBTYPE_ROOT, {
369+
childRules: [{ childType: "adapter", childSubType: "*", childName: "*" }],
370+
});
371+
}
372+
```
373+
374+
A **reference to your custom top-level type resolves package-aware for free** — a
375+
`ReferenceDescriptor` with `targetType: "adapter"` on some other node validates through
376+
the same resolver as `@objectRef` (FQN-exact, else the referrer's package, else
377+
root-level), so you do **not** hand-walk the tree in a `validate` hook.
373378

374379
## See also
375380

server/typescript/packages/cli/src/lib/output.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ export interface GenFileEntry {
2525
export interface GenResultShape {
2626
files: GenFileEntry[];
2727
outDir: string;
28-
dialect: Dialect;
28+
/** Absent for a value-object-only project (no DB code generated → no dialect used). */
29+
dialect: Dialect | undefined;
2930
dryRun: boolean;
3031
warnings: string[];
3132
}
@@ -48,7 +49,8 @@ const GEN_WORDS: Record<GenFileStatus, string> = {
4849

4950
export function formatGenResult(result: GenResultShape, opts: FormatOptions): string {
5051
const symbols = opts.isTTY ? GEN_GLYPHS : GEN_WORDS;
51-
const header = `meta gen${result.dryRun ? " --dry-run" : ""}${result.dialect}, ${result.outDir}`;
52+
// A value-object-only project uses no dialect — show only the outDir in that case.
53+
const header = `meta gen${result.dryRun ? " --dry-run" : ""}${result.dialect ? `${result.dialect}, ` : ""}${result.outDir}`;
5254

5355
if (result.files.length === 0) {
5456
return `${header}\n\n No entities to generate.\n`;

server/typescript/packages/codegen-ts/src/metaobjects-config.ts

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,23 @@ export interface ResolvedGenConfig {
6666
providedEnumModule?: string;
6767
}
6868

69-
export interface MetaobjectsGenConfig extends ResolvedGenConfig {
69+
/** Default dialect / entity-import when a value-object-only project omits them.
70+
* Inert — they are only ever read when DB code is generated, and a project that
71+
* would generate DB code is required to set them explicitly (see `runGen`'s guard). */
72+
export const DEFAULT_DIALECT: Dialect = "sqlite";
73+
export const DEFAULT_DB_IMPORT = "./db";
74+
75+
/**
76+
* The user-facing codegen config. `dbImport` / `dialect` are OPTIONAL here (unlike the
77+
* resolved `ResolvedGenConfig` the generators consume): a value-object-only project
78+
* (no `object.entity` / `object.projection`) generates zero database / query / route
79+
* code, so requiring them would be a dead-but-mandatory `tsc` obligation. `runGen`
80+
* fills inert defaults when they are absent AND the model emits no DB artifacts, and
81+
* throws a clear error when they are absent but the model DOES emit DB code (#194).
82+
*/
83+
export interface MetaobjectsGenConfig extends Omit<ResolvedGenConfig, "dbImport" | "dialect"> {
84+
dbImport?: string;
85+
dialect?: Dialect;
7086
/**
7187
* Generators to run. Each entry is either a typed generator factory result
7288
* (`entityFile()`) or a stable-name string (`"entity"`) resolved via the
@@ -129,7 +145,11 @@ export interface MetaobjectsGenConfig extends ResolvedGenConfig {
129145
* `targets` is Omitted from the base so it can narrow from the user-facing
130146
* TargetConfig to the fully-resolved ResolvedTarget (incompatible under
131147
* exactOptionalPropertyTypes otherwise). */
132-
export interface NormalizedMetaobjectsGenConfig extends Omit<MetaobjectsGenConfig, "targets" | "generators"> {
148+
export interface NormalizedMetaobjectsGenConfig
149+
extends Omit<MetaobjectsGenConfig, "targets" | "generators" | "dbImport" | "dialect"> {
150+
/** Resolved to a concrete value (the user's, else the inert default). */
151+
dbImport: string;
152+
dialect: Dialect;
133153
/** Fully resolved — every string spec has been mapped to its factory result. */
134154
generators: Generator[];
135155
columnNamingStrategy: ColumnNamingStrategy;
@@ -202,7 +222,7 @@ export function resolveTargets(config: MetaobjectsGenConfig): Record<string, Res
202222
outDir: config.outDir,
203223
importBase: config.importBase,
204224
outputLayout: layout,
205-
dbImport: config.dbImport,
225+
dbImport: config.dbImport ?? DEFAULT_DB_IMPORT,
206226
// The default target is the server package — runtime bindings on.
207227
runtime: true,
208228
},
@@ -213,7 +233,7 @@ export function resolveTargets(config: MetaobjectsGenConfig): Record<string, Res
213233
outDir: t.outDir,
214234
importBase: t.importBase,
215235
outputLayout: t.outputLayout ?? layout,
216-
dbImport: t.dbImport ?? config.dbImport,
236+
dbImport: t.dbImport ?? config.dbImport ?? DEFAULT_DB_IMPORT,
217237
runtime: t.runtime ?? true,
218238
};
219239
}
@@ -257,6 +277,8 @@ export function resolveGenerators(specs: readonly GeneratorSpec[]): Generator[]
257277
export function normalizeConfig(config: MetaobjectsGenConfig): NormalizedMetaobjectsGenConfig {
258278
return {
259279
...config,
280+
dbImport: config.dbImport ?? DEFAULT_DB_IMPORT,
281+
dialect: config.dialect ?? DEFAULT_DIALECT,
260282
generators: resolveGenerators(config.generators),
261283
columnNamingStrategy: config.columnNamingStrategy ?? DEFAULT_COLUMN_NAMING_STRATEGY,
262284
pluralizeCollections: config.pluralizeCollections ?? true,

server/typescript/packages/codegen-ts/src/runner.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,30 @@ export async function runGen(opts: RunGenOpts): Promise<RunGenResult> {
149149
return { files: [], warnings, conflicts: [] };
150150
}
151151

152+
// #194 — dbImport / dialect are optional config, BUT a model that emits database
153+
// artifacts (any concrete object that is not an object.value — entities and
154+
// projections generate schema / query / route code) genuinely needs them. Guard
155+
// BEFORE normalizeConfig fills its inert defaults, so a DB project that forgot
156+
// either gets a clear error naming the objects, never silently-defaulted output
157+
// (e.g. a Postgres project quietly emitting sqlite). A value-object-only project
158+
// reaches normalizeConfig and gets the harmless placeholders.
159+
const dbEmittingObjects = safeEntities.filter(
160+
(e) => !e.isAbstract && e.subType !== OBJECT_SUBTYPE_VALUE,
161+
);
162+
if (dbEmittingObjects.length > 0) {
163+
const missing: string[] = [];
164+
if (opts.config.dialect === undefined) missing.push("dialect");
165+
if (opts.config.dbImport === undefined) missing.push("dbImport");
166+
if (missing.length > 0) {
167+
const names = dbEmittingObjects.map((e) => e.name).join(", ");
168+
throw new Error(
169+
`codegen config is missing ${missing.join(" and ")} — required because this model ` +
170+
`generates database code for: ${names}. Set ${missing.join(" and ")} in ` +
171+
`metaobjects.config.ts. (Only a value-object-only model may omit them.)`,
172+
);
173+
}
174+
}
175+
152176
// 2. Resolve targets + entity-module target.
153177
const config = normalizeConfig(opts.config);
154178
const targets = config.targets;

server/typescript/packages/codegen-ts/test/metaobjects-config.test.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, test, expect, expectTypeOf } from "bun:test";
2-
import { defineConfig, normalizeConfig, resolveTargets, DEFAULT_TARGET_NAME, type MetaobjectsGenConfig, type ResolvedGenConfig } from "../src/metaobjects-config.js";
2+
import { defineConfig, normalizeConfig, resolveTargets, DEFAULT_TARGET_NAME, type MetaobjectsGenConfig, type ResolvedGenConfig, type Dialect } from "../src/metaobjects-config.js";
33
import type { Generator } from "../src/generator.js";
44

55
describe("resolveTargets", () => {
@@ -67,9 +67,14 @@ describe("defineConfig", () => {
6767
expectTypeOf<MetaobjectsGenConfig["generators"]>().toEqualTypeOf<(Generator | string)[]>();
6868
});
6969

70-
test("type-level: MetaobjectsGenConfig embeds ResolvedGenConfig (all required fields present, types exact)", () => {
71-
expectTypeOf<Pick<MetaobjectsGenConfig, "outDir" | "extStyle" | "dbImport" | "dialect" | "outputLayout" | "includeHonoRoutes" | "providedEnumModule">>()
72-
.toEqualTypeOf<ResolvedGenConfig>();
70+
test("type-level: MetaobjectsGenConfig embeds ResolvedGenConfig's non-DB fields exactly (#194 — dbImport/dialect are OPTIONAL here, filled by the runner)", () => {
71+
// Everything except dbImport/dialect matches ResolvedGenConfig field-for-field.
72+
expectTypeOf<Pick<MetaobjectsGenConfig, "outDir" | "extStyle" | "outputLayout" | "includeHonoRoutes" | "providedEnumModule">>()
73+
.toEqualTypeOf<Omit<ResolvedGenConfig, "dbImport" | "dialect">>();
74+
// dbImport/dialect are OPTIONAL on the user config (a value-object-only project omits
75+
// them); ResolvedGenConfig (what generators consume) keeps them required.
76+
expectTypeOf<MetaobjectsGenConfig["dbImport"]>().toEqualTypeOf<string | undefined>();
77+
expectTypeOf<MetaobjectsGenConfig["dialect"]>().toEqualTypeOf<Dialect | undefined>();
7378
});
7479
});
7580

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
// #194 item 3 — a value-object-only project may omit dbImport/dialect (it generates
2+
// zero DB code, so requiring them was a dead-but-mandatory tsc obligation). A model
3+
// that DOES emit DB code must still set them, and omitting them errors clearly rather
4+
// than silently defaulting (which would e.g. emit sqlite for a Postgres project).
5+
import { describe, test, expect, beforeEach, afterEach } from "bun:test";
6+
import { mkdtempSync, rmSync } from "node:fs";
7+
import { tmpdir } from "node:os";
8+
import { join } from "node:path";
9+
import { MetaDataLoader, InMemoryStringSource } from "@metaobjectsdev/metadata";
10+
import { runGen } from "../src/runner.js";
11+
import { entityFile } from "../src/generators/entity-file.js";
12+
import type { MetaobjectsGenConfig } from "../src/metaobjects-config.js";
13+
14+
let tmp: string;
15+
beforeEach(() => { tmp = mkdtempSync(join(tmpdir(), "codegen-vo-only-")); });
16+
afterEach(() => { rmSync(tmp, { recursive: true, force: true }); });
17+
18+
async function load(children: unknown[]) {
19+
const json = JSON.stringify({ "metadata.root": { package: "app", children } });
20+
const { root, errors } = await new MetaDataLoader().load([new InMemoryStringSource(json)]);
21+
expect(errors).toEqual([]);
22+
return root;
23+
}
24+
25+
const VALUE_ONLY = [
26+
{ "object.value": { name: "Slots", children: [
27+
{ "field.string": { name: "goal", "@required": true } },
28+
{ "field.string": { name: "note" } },
29+
] } },
30+
];
31+
32+
const ENTITY = [
33+
{ "object.entity": { name: "User", children: [
34+
{ "field.long": { name: "id" } },
35+
{ "field.string": { name: "email", "@required": true } },
36+
{ "identity.primary": { name: "pk", "@fields": ["id"], "@generation": "increment" } },
37+
] } },
38+
];
39+
40+
// dbImport/dialect deliberately OMITTED — legal for a value-object-only model.
41+
// outDir is filled per-test with the beforeEach tmp dir (no writes into the package).
42+
const configNoDb = (): MetaobjectsGenConfig =>
43+
({ outDir: tmp, extStyle: "js", generators: [entityFile()] } as MetaobjectsGenConfig);
44+
45+
describe("#194 item 3 — dbImport/dialect optional for value-object-only projects", () => {
46+
test("a value-object-only model generates with NO dbImport/dialect set (no throw)", async () => {
47+
const root = await load(VALUE_ONLY);
48+
const result = await runGen({ config: configNoDb(), metadata: root, projectRoot: tmp });
49+
// It runs to completion; a value object emits no query/route DB code, so the
50+
// absent dbImport/dialect are never read.
51+
expect(result.warnings.some((w) => w.includes("missing"))).toBe(false);
52+
expect(Array.isArray(result.files)).toBe(true);
53+
});
54+
55+
test("a model with a DB entity but no dbImport/dialect throws a clear error naming the entity", async () => {
56+
const root = await load(ENTITY);
57+
let err: Error | undefined;
58+
try {
59+
await runGen({ config: configNoDb(), metadata: root, projectRoot: tmp });
60+
} catch (e) {
61+
err = e as Error;
62+
}
63+
expect(err).toBeDefined();
64+
expect(err!.message).toContain("dialect and dbImport");
65+
expect(err!.message).toContain("User"); // names the DB-emitting object
66+
});
67+
68+
test("a DB entity WITH dbImport/dialect set generates normally (no regression)", async () => {
69+
const root = await load(ENTITY);
70+
const cfg = { ...configNoDb(), dbImport: "./db", dialect: "postgres" as const } as MetaobjectsGenConfig;
71+
const result = await runGen({ config: cfg, metadata: root, projectRoot: tmp });
72+
expect(result.files.length).toBeGreaterThan(0);
73+
});
74+
});

0 commit comments

Comments
 (0)