Skip to content

Commit 0dbba0e

Browse files
dmealingclaude
andcommitted
fix(codegen-ts): an assigned primary key must be required on insert
An entity whose primary key carries no `identity.primary @generation` -- a natural key, or an id issued by something upstream -- generated code that did not compile. The InsertSchema read a PK field's optionality off `@required`, like any other field, so a PK with no `@required` became `id: z.string().optional()`. The Drizzle column for that same PK is `text("id").primaryKey()` with NO default and is therefore required on insert, and the generated `create<Entity>` pipes `InsertSchema.parse(data)` straight into `.values()`. tsc reported TS2769 ("No overload matches this call") on the queries file. It was a semantic hole as well as a typing one: the schema accepted a create payload with no primary key. The reasoning already existed one function away. `primaryKeyFieldNames` excludes PKs from the UpdateSchema's `.nullable()` treatment because "a PK column is never NULL" -- the same fact says PK optionality can never be read off `@required` alone. This applies it to the insert shape. Narrow by construction. A `@generation: increment|uuid` PK is still omitted from the schema entirely (the caller never supplies it) and a PK with a `@default` stays optional (its column carries that default), so the only shape that moves is the one that could not compile. Output is byte-identical for every model in this repo -- all of them generate their PKs, which is precisely why nothing here caught it; `verify --codegen` on the advanced-modeling example reports no drift. Gated by a compile test that runs the real TypeScript compiler over the entity AND queries files TOGETHER: the defect is a mismatch BETWEEN those two files, so no single-file gate could have seen it. It pins both already-correct arms (generated uuid PK, int rowid PK) against regression, on both dialects. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 550adf2 commit 0dbba0e

3 files changed

Lines changed: 250 additions & 12 deletions

File tree

CHANGELOG.md

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
77

88
## [Unreleased]
99

10+
## [0.22.0] — npm `0.22.0` · PyPI `0.22.0` · NuGet `0.22.0` · Maven `7.22.0`
11+
1012
A **MINOR**: a new registered type family (new vocabulary in all five ports), plus a
1113
registry tightening that turns a previously-permitted provider shape into a hard error.
1214
Nothing here changes runtime behaviour on an existing database.
@@ -127,6 +129,35 @@ structurally.
127129
The conformance corpus's committed canonical schema is regenerated: a parent-side
128130
`relationship.composition` was contributing no referential action.
129131

132+
### Fixed — an assigned primary key generated code that did not compile
133+
134+
**Generated-output change — regenerate to pick it up; three-way merge preserves hand edits.**
135+
136+
An entity whose primary key carries no `identity.primary @generation` — a natural key, or an
137+
id issued by something upstream — emitted a `<Entity>InsertSchema` that made the PK
138+
`.optional()`, because a PK's optionality was read off `@required` like any other field's.
139+
The Drizzle column for that same PK is `text("id").primaryKey()` with **no default**, so it
140+
is required on insert, and the generated `create<Entity>` pipes `InsertSchema.parse(data)`
141+
straight into `.values()`. The result did not typecheck (`TS2769`), and the schema also
142+
accepted a create payload carrying no primary key at all.
143+
144+
An assigned PK with no `@default` is now required in the insert shape. Unchanged: a
145+
`@generation: increment|uuid` PK is still omitted from the schema entirely (the caller never
146+
supplies it), a PK with a `@default` stays optional (the column has that default), and the
147+
`UpdateSchema` keeps every field optional under PATCH semantics.
148+
149+
Output is byte-identical for any model whose PKs are generated or `@required` — which is
150+
every model in this repository, and why nothing here caught it. Gated by a new compile test
151+
that runs the real TypeScript compiler over the entity **and** queries files together, since
152+
the defect was a mismatch *between* the two and no single-file gate could see it.
153+
154+
### Fixed — `@metaobjectsdev/cli` no longer pulls a `yaml` package it never imports
155+
156+
`yaml` entered the CLI's runtime `dependencies` while a requirement was still a YAML side
157+
file the CLI parsed itself. Requirements became registered vocabulary two commits later and
158+
the only file importing it was deleted; the dependency was not. Nothing in the package
159+
imports the module today, so an install of the CLI no longer resolves it.
160+
130161

131162
## [0.21.6] — npm `0.21.6` · PyPI `0.21.6` · NuGet `0.21.6` · Maven `7.21.6`
132163

server/typescript/packages/codegen-ts/src/templates/zod-validators.ts

Lines changed: 75 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,44 @@ function primaryKeyFieldNames(obj: MetaObject): Set<string> {
159159
return new Set(primaryIdentityFieldNames(obj));
160160
}
161161

162+
/** PK field names the CALLER must supply on insert: an ASSIGNED primary key —
163+
* a natural key or an externally-issued id, i.e. a primary identity carrying no
164+
* @generation — that also has no @default to fill it.
165+
*
166+
* These are insert-REQUIRED regardless of @required, and that is a typing
167+
* necessity rather than a policy choice: the Drizzle column for such a PK is
168+
* `text("id").primaryKey()` with no default, so its inferred insert type
169+
* requires the value. Deriving the PK's optionality from @required like any
170+
* other field emitted `id: z.string().optional()`, and the generated
171+
* `create<Entity>` — which pipes `InsertSchema.parse(data)` straight into
172+
* `.values()` — then failed to compile (TS2769). It was also a semantic hole:
173+
* the schema accepted a create payload with no primary key at all.
174+
*
175+
* Excluded, because each is already insert-optional for a real reason:
176+
* • @generation increment|uuid PKs — omitted from the schema entirely
177+
* (autoGenPkFieldNames), the caller never supplies them;
178+
* • a PK carrying @default — the column gets that default, so omitting is
179+
* legal and forcing it required would make callers repeat the default.
180+
* Mirrors primaryKeyFieldNames' reasoning for the UpdateSchema: a PK column is
181+
* never NULL, so PK optionality can never be read off @required alone. */
182+
function assignedPkFieldNames(obj: MetaObject): Set<string> {
183+
const autoGen = autoGenPkFieldNames(obj);
184+
const out = new Set<string>();
185+
for (const name of primaryIdentityFieldNames(obj)) {
186+
if (autoGen.has(name)) continue;
187+
out.add(name);
188+
}
189+
return out;
190+
}
191+
192+
/** True when this field is an assigned PK with no @default — see
193+
* assignedPkFieldNames. Kept as one predicate so the emitters and the
194+
* documented-shape function (insertSchemaFields) cannot drift. */
195+
function isInsertRequiredPk(field: MetaField, assignedPk: Set<string>): boolean {
196+
if (!assignedPk.has(field.name)) return false;
197+
return field.attr(FIELD_ATTR_DEFAULT) === undefined;
198+
}
199+
162200
/**
163201
* Emit ONLY the `<Name>InsertSchema`. Used by the value-object file emitter
164202
* for metaobjects with no writable source.rdb — those have no PATCH/update
@@ -171,6 +209,7 @@ function primaryKeyFieldNames(obj: MetaObject): Set<string> {
171209
export function renderInsertSchemaOnly(obj: MetaObject, ctx?: RenderContext): Code {
172210
const z = imp("z@zod");
173211
const autoGenPkFields = autoGenPkFieldNames(obj);
212+
const assignedPkFields = assignedPkFieldNames(obj);
174213
const tphPin = tphDiscriminatorPin(obj);
175214

176215
const insertFieldLines: Code[] = [];
@@ -204,7 +243,9 @@ export function renderInsertSchemaOnly(obj: MetaObject, ctx?: RenderContext): Co
204243
: code` ${child.name}: z.string().optional().transform(() => new Date().toISOString())`,
205244
);
206245
} else {
207-
insertFieldLines.push(code` ${child.name}: ${zodFieldExpr(child, obj, ctx)}`);
246+
insertFieldLines.push(
247+
code` ${child.name}: ${zodFieldExpr(child, obj, ctx, isInsertRequiredPk(child, assignedPkFields))}`,
248+
);
208249
}
209250
}
210251

@@ -246,11 +287,14 @@ export interface SchemaFieldShape {
246287
* • @readOnly fields are omitted (DB / replication owns the write path);
247288
* • a TPH subtype's @discriminator field is a pinned `z.literal(value)`;
248289
* • @autoSet fields are present but optional (server fills them);
290+
* • an ASSIGNED primary key with no @default is required (see
291+
* assignedPkFieldNames — its Drizzle column has no default);
249292
* • every other field's optionality is `fieldWillBeOptional` (not required, or
250293
* carries a @default).
251294
*/
252295
export function insertSchemaFields(obj: MetaObject): SchemaFieldShape[] {
253296
const autoGenPkFields = autoGenPkFieldNames(obj);
297+
const assignedPkFields = assignedPkFieldNames(obj);
254298
const tphPin = tphDiscriminatorPin(obj);
255299
const out: SchemaFieldShape[] = [];
256300
for (const child of obj.fields()) {
@@ -266,7 +310,10 @@ export function insertSchemaFields(obj: MetaObject): SchemaFieldShape[] {
266310
if (autoSet === AUTO_SET_ON_CREATE || autoSet === AUTO_SET_ON_UPDATE) {
267311
out.push({ name: child.name, optional: true, autoSet: true });
268312
} else {
269-
out.push({ name: child.name, optional: fieldWillBeOptional(child) });
313+
out.push({
314+
name: child.name,
315+
optional: isInsertRequiredPk(child, assignedPkFields) ? false : fieldWillBeOptional(child),
316+
});
270317
}
271318
}
272319
return out;
@@ -311,6 +358,7 @@ export function renderZodValidators(obj: MetaObject, ctx?: RenderContext): Code
311358
const z = imp("z@zod");
312359
const autoGenPkFields = autoGenPkFieldNames(obj);
313360
const pkFields = primaryKeyFieldNames(obj);
361+
const assignedPkFields = assignedPkFieldNames(obj);
314362
const tphPin = tphDiscriminatorPin(obj);
315363

316364
const insertFieldLines: Code[] = [];
@@ -358,7 +406,9 @@ export function renderZodValidators(obj: MetaObject, ctx?: RenderContext): Code
358406
// field expr) so an import/restore keeps the caller's original timestamp.
359407
preservingFieldLines.push(code` ${child.name}: ${zodFieldExpr(child, obj, ctx)}`);
360408
} else {
361-
const fieldLine = code` ${child.name}: ${zodFieldExpr(child, obj, ctx)}`;
409+
// The preserving schema is an INSERT shape (import / restore / replication),
410+
// so an assigned PK is required there for the same typing reason.
411+
const fieldLine = code` ${child.name}: ${zodFieldExpr(child, obj, ctx, isInsertRequiredPk(child, assignedPkFields))}`;
362412
insertFieldLines.push(fieldLine);
363413
preservingFieldLines.push(fieldLine);
364414
}
@@ -452,7 +502,14 @@ function zodScalarFor(subType: string): string {
452502
return "z.string()"; // string/uuid/date/time/timestamp/decimal/enum on the wire
453503
}
454504

455-
function zodFieldExpr(field: MetaField, owner?: MetaObject, ctx?: RenderContext): Code {
505+
function zodFieldExpr(
506+
field: MetaField,
507+
owner?: MetaObject,
508+
ctx?: RenderContext,
509+
/** Suppress the trailing `.optional()` — see assignedPkFieldNames. Set ONLY
510+
* by the insert-shape emitters; the update/read shapes must stay optional. */
511+
forceRequired = false,
512+
): Code {
456513
// `@dbColumnType: jsonb` on a scalar (legal only on field.string) is the
457514
// sanctioned "open JSON bag" escape hatch — a genuinely untyped JSON column
458515
// with no value-object to reference. Its Drizzle column is a bare `jsonb()`,
@@ -465,7 +522,7 @@ function zodFieldExpr(field: MetaField, owner?: MetaObject, ctx?: RenderContext)
465522
if (field.attr(FIELD_ATTR_DB_COLUMN_TYPE) === DB_COLUMN_TYPE_JSONB) {
466523
let base: Code = code`z.unknown()`;
467524
if (field.resolvedIsArray()) base = code`z.array(${base})`;
468-
return appendValidatorChain(base, field);
525+
return appendValidatorChain(base, field, forceRequired);
469526
}
470527

471528
// FIELD_SUBTYPE_OBJECT: emit z.array(<Ref>InsertSchema) / <Ref>InsertSchema
@@ -490,13 +547,13 @@ function zodFieldExpr(field: MetaField, owner?: MetaObject, ctx?: RenderContext)
490547
const refImp = imp(`${refName}InsertSchema@${moduleSpec}`);
491548
let base: Code = code`${refImp}`;
492549
if (field.resolvedIsArray()) base = code`z.array(${base})`;
493-
return appendValidatorChain(base, field);
550+
return appendValidatorChain(base, field, forceRequired);
494551
}
495552
// No resolvable @objectRef — fall through to z.unknown(); downstream code
496553
// can still pass a value through but loses validation.
497554
let base: Code = code`z.unknown()`;
498555
if (field.resolvedIsArray()) base = code`z.array(${base})`;
499-
return appendValidatorChain(base, field);
556+
return appendValidatorChain(base, field, forceRequired);
500557
}
501558

502559
// field.map → z.record(z.string(), V): value is a VO's InsertSchema (@objectRef)
@@ -509,10 +566,10 @@ function zodFieldExpr(field: MetaField, owner?: MetaObject, ctx?: RenderContext)
509566
? valueObjectModuleSpecifier(refName, ctx.packageOf, owner.package, ctx.outputLayout, ctx.extStyle)
510567
: `./${refName}.js`;
511568
const refImp = imp(`${refName}InsertSchema@${moduleSpec}`);
512-
return appendValidatorChain(code`z.record(z.string(), ${refImp})`, field);
569+
return appendValidatorChain(code`z.record(z.string(), ${refImp})`, field, forceRequired);
513570
}
514571
const vt = field.attr(FIELD_ATTR_VALUE_TYPE);
515-
return appendValidatorChain(code`z.record(z.string(), ${zodScalarFor(typeof vt === "string" ? vt : "string")})`, field);
572+
return appendValidatorChain(code`z.record(z.string(), ${zodScalarFor(typeof vt === "string" ? vt : "string")})`, field, forceRequired);
516573
}
517574

518575
let baseStr: string;
@@ -573,7 +630,7 @@ function zodFieldExpr(field: MetaField, owner?: MetaObject, ctx?: RenderContext)
573630
const sharedConst = imp(`${constName}@${spec}`);
574631
let base: Code = code`${sharedConst}`;
575632
if (field.resolvedIsArray()) base = code`z.array(${base})`;
576-
return appendValidatorChain(base, field);
633+
return appendValidatorChain(base, field, forceRequired);
577634
}
578635
}
579636
baseStr = zodEnumExpr(values);
@@ -612,7 +669,7 @@ function zodFieldExpr(field: MetaField, owner?: MetaObject, ctx?: RenderContext)
612669
}
613670

614671
if (field.resolvedIsArray()) baseStr = `z.array(${baseStr})`;
615-
return appendValidatorChain(code`${baseStr}`, field);
672+
return appendValidatorChain(code`${baseStr}`, field, forceRequired);
616673
}
617674

618675
/** Mirrors the optional-or-not decision inside appendValidatorChain so the update-schema
@@ -635,7 +692,7 @@ const NUMERIC_FIELD_SUBTYPES = new Set<string>([
635692
* - numeric (scalar) → .min/.max = numeric value (validator.numeric)
636693
* - array (any element) → .min/.max = element count (validator.array)
637694
*/
638-
function appendValidatorChain(base: Code, field: MetaField): Code {
695+
function appendValidatorChain(base: Code, field: MetaField, forceRequired = false): Code {
639696
let isRequired = field.attr(FIELD_ATTR_REQUIRED) === true;
640697
let maxLen: number | undefined = field.attr(FIELD_ATTR_MAX_LENGTH) as number | undefined;
641698
let minLen: number | undefined;
@@ -709,6 +766,12 @@ function appendValidatorChain(base: Code, field: MetaField): Code {
709766
if (numMax !== undefined) chain = code`${chain}.max(${numMax})`;
710767
}
711768

769+
// An assigned PK is insert-REQUIRED whatever @required says — its Drizzle
770+
// column has no default, so an optional value cannot typecheck into
771+
// `.values()`. Only the caller knows this (the update/read shapes must stay
772+
// optional), hence the explicit opt-in. See assignedPkFieldNames.
773+
if (forceRequired) return chain;
774+
712775
// Fields with DB-level defaults are optional in the InsertSchema: the caller
713776
// can omit them and the DB will fill in. Otherwise required-with-default
714777
// would force callers to repeat the default at every call site.
Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
// Regression guard: an ASSIGNED primary key (an entity whose PK carries no
2+
// `identity.primary @generation`, e.g. a natural key or an externally-issued id)
3+
// emitted generated code that did not compile.
4+
//
5+
// The InsertSchema derived a PK field's optionality from `@required` like any
6+
// other field, so a PK with no `@required` became `id: z.string().optional()`.
7+
// The Drizzle column for that same PK is `text("id").primaryKey()` — no default,
8+
// therefore REQUIRED on insert. `createOrder` pipes one straight into the other
9+
// (`db.insert(t).values(InsertSchema.parse(data))`), so `tsc` reported TS2769
10+
// ("No overload matches this call") on the generated queries file.
11+
//
12+
// Two arms are already correct and are pinned here so a fix can't regress them:
13+
// • a GENERATED pk (@generation: uuid|increment) is OMITTED from the
14+
// InsertSchema entirely — the caller never supplies it;
15+
// • an int rowid pk compiles even when optional, because Drizzle's
16+
// `integer().primaryKey()` is rowid-aliased and thus insert-optional.
17+
//
18+
// This compiles entity + queries TOGETHER with the real TS compiler — the bug is
19+
// a mismatch BETWEEN those two files, so a single-file gate cannot see it.
20+
21+
import { describe, test, expect } from "bun:test";
22+
import { mkdtempSync, writeFileSync, rmSync } from "node:fs";
23+
import { join } from "node:path";
24+
import ts from "typescript";
25+
import { MetaDataLoader, InMemoryStringSource } from "@metaobjectsdev/metadata";
26+
import { entityFile, queriesFile } from "../src/generators/index.js";
27+
import { makeRenderContext } from "../src/render-context.js";
28+
import { buildPkMap } from "../src/pk-resolver.js";
29+
import { buildRelationMap } from "../src/relation-resolver.js";
30+
import type { GenContext } from "../src/generator.js";
31+
import type { Dialect } from "../src/metaobjects-config.js";
32+
33+
/** @param pkField the PK field node; @param identity the identity.primary node. */
34+
async function loadRoot(pkField: unknown, identity: unknown) {
35+
const json = JSON.stringify({
36+
"metadata.root": {
37+
package: "test",
38+
children: [
39+
{
40+
"object.entity": {
41+
name: "Order",
42+
children: [
43+
{ "source.rdb": { "@kind": "table", "@table": "orders" } },
44+
pkField,
45+
{ "field.string": { name: "buyer", "@required": true } },
46+
identity,
47+
],
48+
},
49+
},
50+
],
51+
},
52+
});
53+
const result = await new MetaDataLoader().load([new InMemoryStringSource(json)]);
54+
if (result.errors.length > 0) {
55+
throw new Error(`Loader errors:\n${result.errors.map((e) => e.message).join("\n")}`);
56+
}
57+
return result.root;
58+
}
59+
60+
// Real TS compiler gate. Files land under this package so the program resolves
61+
// drizzle-orm / zod from the package's own node_modules (the REAL types).
62+
function compile(dir: string, files: string[]): readonly ts.Diagnostic[] {
63+
const program = ts.createProgram(
64+
files.map((f) => join(dir, f)),
65+
{
66+
strict: true,
67+
noEmit: true,
68+
target: ts.ScriptTarget.ES2022,
69+
module: ts.ModuleKind.ESNext,
70+
moduleResolution: ts.ModuleResolutionKind.Bundler,
71+
skipLibCheck: true,
72+
},
73+
);
74+
return ts.getPreEmitDiagnostics(program);
75+
}
76+
77+
async function genAndCompile(
78+
dialect: Dialect,
79+
pkField: unknown,
80+
identity: unknown,
81+
): Promise<string[]> {
82+
const root = await loadRoot(pkField, identity);
83+
const dir = mkdtempSync(join(import.meta.dir, "tmp-assigned-pk-"));
84+
try {
85+
const renderContext = makeRenderContext({
86+
dialect,
87+
loadedRoot: root,
88+
outDir: dir,
89+
dbImport: "~/db",
90+
pkMap: buildPkMap(root),
91+
relationMap: buildRelationMap(root),
92+
});
93+
const ctx: GenContext = {
94+
entities: root.objects(),
95+
loadedRoot: root,
96+
matches: () => true,
97+
projectRoot: dir,
98+
config: { outDir: dir, extStyle: "none", dbImport: "~/db", dialect } as never,
99+
renderContext,
100+
warn: () => {},
101+
};
102+
// The queries file declares its own `Db` type from drizzle, so entity+queries
103+
// is a self-contained program — no dbImport module needs to resolve.
104+
const files = [
105+
...(await entityFile({ allowlists: false }).generate(ctx)),
106+
...(await queriesFile().generate(ctx)),
107+
];
108+
for (const f of files) writeFileSync(join(dir, f.path), f.content);
109+
return compile(dir, files.map((f) => f.path)).map((d) =>
110+
ts.flattenDiagnosticMessageText(d.messageText, "\n"),
111+
);
112+
} finally {
113+
rmSync(dir, { recursive: true, force: true });
114+
}
115+
}
116+
117+
const ASSIGNED_PK = { "identity.primary": { name: "pk", "@fields": "id" } };
118+
const UUID_GEN_PK = { "identity.primary": { name: "pk", "@fields": "id", "@generation": "uuid" } };
119+
const INCR_PK = { "identity.primary": { name: "pk", "@fields": "id", "@generation": "increment" } };
120+
121+
const UUID_FIELD = { "field.uuid": { name: "id" } };
122+
const STRING_FIELD = { "field.string": { name: "id" } };
123+
const INT_FIELD = { "field.long": { name: "id" } };
124+
125+
describe("assigned (non-generated) PK emits typechecking insert code (TS2769 guard)", () => {
126+
for (const dialect of ["postgres", "sqlite"] as const) {
127+
test(`${dialect}: uuid PK with NO @generation compiles`, async () => {
128+
expect(await genAndCompile(dialect, UUID_FIELD, ASSIGNED_PK)).toEqual([]);
129+
});
130+
131+
test(`${dialect}: string natural-key PK with NO @generation compiles`, async () => {
132+
expect(await genAndCompile(dialect, STRING_FIELD, ASSIGNED_PK)).toEqual([]);
133+
});
134+
135+
// Already-correct arms, pinned so the fix cannot regress them.
136+
test(`${dialect}: uuid PK with @generation: uuid compiles (PK omitted from InsertSchema)`, async () => {
137+
expect(await genAndCompile(dialect, UUID_FIELD, UUID_GEN_PK)).toEqual([]);
138+
});
139+
140+
test(`${dialect}: int PK with @generation: increment compiles`, async () => {
141+
expect(await genAndCompile(dialect, INT_FIELD, INCR_PK)).toEqual([]);
142+
});
143+
}
144+
});

0 commit comments

Comments
 (0)