diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index af2d756a3..c456c0118 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -180,9 +180,16 @@ jobs: # catches it. run: cd server && npm run typecheck:tests - - name: Typecheck scripts + - name: Typecheck entity generator + # tsconfig.scripts.json only covers scripts/generate-entities.ts — + # gen-plugin-facts.ts/migrate-encryption.ts don't compile under this + # config (import.meta needs a newer --module target), so the gate is + # named for what it actually covers rather than the whole directory. run: cd server && npm run typecheck:scripts + - name: Entity files match the generator + run: cd server && npm run check:entities + - name: Lint run: cd server && npm run lint:check diff --git a/server/package.json b/server/package.json index 06bf0eb62..3ab107e08 100644 --- a/server/package.json +++ b/server/package.json @@ -15,6 +15,7 @@ "gen:plugin-facts": "node --import tsx scripts/gen-plugin-facts.ts", "check:plugin-facts": "node --import tsx scripts/gen-plugin-facts.ts --check", "gen:entities": "node --import tsx scripts/generate-entities.ts", + "check:entities": "node --import tsx scripts/generate-entities.ts --check", "format": "prettier --write \"src/**/*.ts\" \"test/**/*.ts\"", "format:check": "prettier --check \"src/**/*.ts\" \"test/**/*.ts\"", "lint": "eslint \"{src,apps,libs,test}/**/*.ts\" --fix", diff --git a/server/scripts/generate-entities.ts b/server/scripts/generate-entities.ts index 7e17be1a9..75556c9c5 100644 --- a/server/scripts/generate-entities.ts +++ b/server/scripts/generate-entities.ts @@ -321,6 +321,58 @@ export function RULE1c_markBooleanColumns(metadata: EntityMetadata[]): RetypedSc return fixups; } +/** + * Rule 1d (metadata level): a numeric scalar's class-field initialiser must + * never render `NaN`. + * + * `@mikro-orm/sql`'s own introspection (`DatabaseTable.js#getPropertyDeclaration` + * -> `getPropertyDefaultValue`) computes a NUMERIC column's `prop.default` as + * `+defaultValue` — a plain unary-plus coercion of the raw SQLite default + * text — whenever that text doesn't equal the literal, lowercase string + * `'null'`. SQLite reports the DDL's exact casing back through + * `PRAGMA table_info` (`'NULL'`, not `'null'`, for `DEFAULT NULL`), and + * `SchemaHelper#normalizeDefaultValue`'s own `'null'` check + * (`DatabaseTable.js` line ~869) is the same case-sensitive comparison — so + * neither the `DEFAULT NULL` case NOR a non-literal expression default + * (`DEFAULT (strftime('%s','now'))`) is ever recognised as "no usable + * numeric value", and `+'NULL'` / `+"(strftime('%s','now'))"` both evaluate + * to `NaN`. That `NaN` becomes `prop.default`, which + * `SourceFile.js#getPropertyDefinition` then treats as a genuine, usable + * default (`NaN` is neither `undefined` nor `null`) and renders `= NaN` as + * the class-field initialiser — confirmed by generating the real schema + * (`BudgetItems.persons`/`.days`, `IdempotencyKeys.created_at`, + * `BudgetItems.place_id`/`.reservation_id`, `PackingBags.user_id`) and by + * reading `node_modules/@mikro-orm/sql/schema/DatabaseTable.js`, not + * assumed. `prop.defaultRaw` itself is unaffected — it is threaded straight + * through to `.defaultRaw(...)` in the rendered schema and is correct there; + * only the class-field initialiser is wrong, and it is wrong specifically + * because the upstream library's `NaN` looks like "a real number" to the + * renderer's own `useDefault` check. + * + * Clearing `prop.default` back to `undefined` (never `null` — `null` would + * make the renderer's `hasUsableNullDefault` branch fire and print `= null`, + * which is still an initialiser this rule must not emit) makes the + * renderer's `useDefault` check naturally false, so NO initialiser is + * emitted at all: the class field falls back to nullability alone — + * `?: number | null` for a nullable column, `!: number` for a NOT NULL one + * (matching every other scalar's fallback shape in this generator). + * + * A `defaultRaw` that IS a finite numeric literal (`'0'`, `'1'`) is left + * completely alone: `Number(raw)` is correct there, `prop.default` already + * holds the right value, and nothing needs fixing. + */ +export function RULE1d_fixNaNNumericDefaults(metadata: EntityMetadata[]): void { + for (const meta of metadata) { + for (const prop of Object.values(meta.properties)) { + if (prop.kind !== undefined && prop.kind !== ReferenceKind.SCALAR) continue; + if (typeof prop.defaultRaw !== 'string') continue; + if (Number.isFinite(Number(prop.defaultRaw))) continue; // a genuine numeric literal — untouched + if (typeof prop.default !== 'number' || !Number.isNaN(prop.default)) continue; // nothing broken here + prop.default = undefined; + } + } +} + // --------------------------------------------------------------------------- // Rule 2 (metadata level): p.datetime() -> p.type(DbTimestampType). // @@ -691,6 +743,7 @@ export function applyRules(metadata: EntityMetadata[], _platform: Platform): Rul // `retypedScalars` (stripRedundantColumnType's "found no .columnType() to // strip" throw catches this immediately if that ever stops being true). RULE1c_markBooleanColumns(metadata); + RULE1d_fixNaNNumericDefaults(metadata); const timestamps = RULE2_datetimeToDbTimestampType(metadata); RULE3_integerPkAutoincrement(metadata); RULE4_hideAllRelations(metadata); @@ -1258,13 +1311,8 @@ export function writeRepositoriesIfMissing(repositoriesDir: string, files: Reado return created; } -/** Regenerates `src/db/entities/index.ts` from the directory, sorted, exporting `ALL_ENTITIES`. */ -export function regenerateEntitiesIndex(entitiesDir: string): string { - const files = fs - .readdirSync(entitiesDir) - .filter((f) => f.endsWith('.entity.ts')) - .sort(); - const classNames = files.map((f) => f.replace(/\.entity\.ts$/, '')); +/** Builds `src/db/entities/index.ts`'s source for a given, already-sorted class-name list. */ +export function buildEntitiesIndexSource(classNames: readonly string[]): string { const imports = classNames.map((name) => `import { ${name}Schema } from './${name}.entity';`).join('\n'); const exportList = classNames.map((name) => ` ${name}Schema,`).join('\n'); return ( @@ -1273,6 +1321,58 @@ export function regenerateEntitiesIndex(entitiesDir: string): string { ); } +/** Regenerates `src/db/entities/index.ts` from the directory, sorted, exporting `ALL_ENTITIES`. */ +export function regenerateEntitiesIndex(entitiesDir: string): string { + const classNames = fs + .readdirSync(entitiesDir) + .filter((f) => f.endsWith('.entity.ts')) + .sort() + .map((f) => f.replace(/\.entity\.ts$/, '')); + return buildEntitiesIndexSource(classNames); +} + +/** What `--check` found: every generated file (entity + `index.ts`) that differs from — or is missing from — disk, and every repository `--write` would create. Read-only; never touches disk. */ +export interface CheckReport { + /** `X.entity.ts` (or `'index.ts'`) for every generated file whose on-disk content differs, byte-for-byte, from what the generator produces now. */ + differingFiles: string[]; + /** `src/db/repositories/.repository.ts` for every entity with no repository file yet — `--write` would create these. */ + missingRepositories: string[]; +} + +/** + * The read-only half of `--check` (I1 in `task-4-review-gates.md`): compares + * `files` (already generated in memory by `generateEntities()`) plus the + * `index.ts` they imply against `entitiesDir`, and reports which + * `repositoriesDir/.repository.ts` files `writeRepositoriesIfMissing` + * would create. Never reads `regenerateEntitiesIndex`'s directory-listing + * variant — the *expected* index is built straight from `files`' own key + * set, not from whatever happens to already be on disk, so a stray or + * missing entity file cannot mask itself out of the index comparison. + */ +export function checkEntities(entitiesDir: string, repositoriesDir: string, files: ReadonlyMap): CheckReport { + const differingFiles: string[] = []; + for (const [fileName, content] of files) { + const onDiskPath = path.join(entitiesDir, fileName); + const onDisk = fs.existsSync(onDiskPath) ? fs.readFileSync(onDiskPath, 'utf8') : undefined; + if (onDisk !== content) differingFiles.push(fileName); + } + + const classNames = [...files.keys()].map((f) => f.replace(/\.entity\.ts$/, '')).sort(); + const expectedIndex = buildEntitiesIndexSource(classNames); + const indexPath = path.join(entitiesDir, 'index.ts'); + const onDiskIndex = fs.existsSync(indexPath) ? fs.readFileSync(indexPath, 'utf8') : undefined; + if (onDiskIndex !== expectedIndex) differingFiles.push('index.ts'); + + const missingRepositories: string[] = []; + for (const fileName of files.keys()) { + const className = fileName.replace(/\.entity\.ts$/, ''); + const repoFile = path.join(repositoriesDir, `${className}.repository.ts`); + if (!fs.existsSync(repoFile)) missingRepositories.push(repoFile); + } + + return { differingFiles, missingRepositories }; +} + // --------------------------------------------------------------------------- // CLI // --------------------------------------------------------------------------- @@ -1280,11 +1380,28 @@ export function regenerateEntitiesIndex(entitiesDir: string): string { async function main(): Promise { const args = process.argv.slice(2); const write = args.includes('--write'); + const check = args.includes('--check'); const outIndex = args.indexOf('--out'); const outDir = outIndex >= 0 ? args[outIndex + 1] : undefined; const { files } = await generateEntities(); + if (check) { + const { differingFiles, missingRepositories } = checkEntities(ENTITIES_DIR, REPOSITORIES_DIR, files); + for (const fileName of differingFiles) { + console.error(`[generate-entities] DRIFT: src/db/entities/${fileName} does not match the generator.`); + } + for (const repoFile of missingRepositories) { + console.error(`[generate-entities] MISSING: ${path.relative(SERVER_ROOT, repoFile)} would be created by --write.`); + } + if (differingFiles.length > 0 || missingRepositories.length > 0) { + console.error('[generate-entities] --check failed. Run: node --import tsx scripts/generate-entities.ts --write'); + process.exit(1); + } + console.log('[generate-entities] --check: entity files match the generator'); + return; + } + if (write) { dumpEntities(ENTITIES_DIR, files); const created = writeRepositoriesIfMissing(REPOSITORIES_DIR, files); diff --git a/server/src/db/entities/BudgetItems.entity.ts b/server/src/db/entities/BudgetItems.entity.ts index fa89de614..562c2b315 100644 --- a/server/src/db/entities/BudgetItems.entity.ts +++ b/server/src/db/entities/BudgetItems.entity.ts @@ -16,8 +16,8 @@ export class BudgetItems { category: string & Opt = 'Other'; name!: string; total_price!: number & Opt; - persons?: number | null = NaN; - days?: number | null = NaN; + persons?: number | null; + days?: number | null; note?: string | null; sort_order?: number | null = 0; created_at?: string | null; @@ -25,12 +25,12 @@ export class BudgetItems { paid_by_user_id?: number | null; expense_date?: string | null; reservation?: Ref | null; - reservation_id?: number | null = NaN; + reservation_id?: number | null; currency?: string | null; exchange_rate!: number & Opt; ticket_json?: string | null; place?: Ref | null; - place_id?: number | null = NaN; + place_id?: number | null; budget_item_members_collection = new Collection(this); budget_item_payers_collection = new Collection(this); file_links_collection = new Collection(this); diff --git a/server/src/db/entities/IdempotencyKeys.entity.ts b/server/src/db/entities/IdempotencyKeys.entity.ts index 70811fae4..543676155 100644 --- a/server/src/db/entities/IdempotencyKeys.entity.ts +++ b/server/src/db/entities/IdempotencyKeys.entity.ts @@ -11,7 +11,7 @@ export class IdempotencyKeys { path!: string; status_code!: number; response_body!: string; - created_at: number & Opt = NaN; + created_at!: number & Opt; } export const IdempotencyKeysSchema = defineEntity({ diff --git a/server/src/db/entities/PackingBags.entity.ts b/server/src/db/entities/PackingBags.entity.ts index b12735ba3..66c60593b 100644 --- a/server/src/db/entities/PackingBags.entity.ts +++ b/server/src/db/entities/PackingBags.entity.ts @@ -15,7 +15,7 @@ export class PackingBags { sort_order?: number | null = 0; created_at?: string | null; user?: Ref | null; - user_id?: number | null = NaN; + user_id?: number | null; packing_bag_members = new Collection(this); packing_items_collection = new Collection(this); } diff --git a/server/tests/unit/db/entity-class-fields.test.ts b/server/tests/unit/db/entity-class-fields.test.ts new file mode 100644 index 000000000..b7a5ed698 --- /dev/null +++ b/server/tests/unit/db/entity-class-fields.test.ts @@ -0,0 +1,142 @@ +import { ReferenceKind } from '@mikro-orm/core'; +import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'; +import { ALL_ENTITIES } from '../../../src/db/entities'; +import { BudgetItems } from '../../../src/db/entities/BudgetItems.entity'; +import { IdempotencyKeys } from '../../../src/db/entities/IdempotencyKeys.entity'; +import { createUser, createTrip } from '../../helpers/factories'; +import { createSnapshotTestDb } from '../../helpers/db-mock'; +import { resetTestDb } from '../../helpers/test-db'; +import { createTestOrm, type TestOrm } from '../../helpers/test-orm'; + +/** + * Permanent guard for `task-4-review-shape.md` Important finding 1: a + * numeric class-field initialiser must never render `NaN` (see + * `scripts/generate-entities.ts`'s `RULE1d_fixNaNNumericDefaults` and + * `tests/unit/db/generate-entities.test.ts`'s `RULE1D-*` fixture tests for + * the mechanism). This file pins the OUTPUT of that rule against the real, + * committed entities — every one of the 125, not a sample — and reproduces + * the reviewer's own insert probe so a future regenerate/hand-edit that + * reintroduces a `NaN` initialiser fails loudly here, not silently at a + * caller's first `em.create()`. + */ +describe('entity class fields never carry a NaN initialiser (task-4-review-shape.md, Important 1)', () => { + it('CLASSFIELD-001: new X() has no own property whose value is NaN, across all 125 entities', () => { + const failures: string[] = []; + for (const schema of ALL_ENTITIES) { + const Ctor = schema.class as new () => Record; + const instance = new Ctor(); + for (const [key, value] of Object.entries(instance)) { + if (typeof value === 'number' && Number.isNaN(value)) { + failures.push(`${schema.meta.className}.${key}`); + } + } + } + expect(failures).toEqual([]); + }); + + /** + * Report-only (per the brief): a NOT NULL scalar property with no + * `default`/`defaultRaw` at all legitimately has no class-field + * initialiser — the caller is expected to supply it (`name!: string`, for + * example). That is normal and not a bug, so this never asserts the list + * is empty; it exists so a reviewer can see the shape at a glance and so a + * suspicious spike in the count (e.g. every numeric property in an entity + * suddenly losing its default) shows up in test output without failing + * the suite on a false positive. + */ + it('CLASSFIELD-002 (report-only): NOT NULL scalar properties with no default and no class-field initialiser', async () => { + const testDb = createSnapshotTestDb(); + try { + const t = await createTestOrm(testDb); + try { + const report: string[] = []; + for (const schema of ALL_ENTITIES) { + const meta = t.orm.getMetadata().get(schema.class); + const Ctor = schema.class as new () => Record; + const instance = new Ctor(); + for (const prop of meta.props) { + if (prop.kind !== undefined && prop.kind !== ReferenceKind.SCALAR) continue; + if (prop.nullable) continue; + if (prop.default !== undefined || prop.defaultRaw !== undefined) continue; + if (!(prop.name in instance)) continue; + if (instance[prop.name] === undefined) { + report.push(`${meta.className}.${prop.name}`); + } + } + } + // Informational only — see the doc comment above. `console.info` (not + // a `console.log` vitest might buffer away) so it survives a run. + console.info(`[CLASSFIELD-002] ${report.length} NOT NULL, no-default scalar propert${report.length === 1 ? 'y' : 'ies'} with no initialiser (expected, not a failure)`); + expect(report).toEqual(report); // always true — this test only ever reports, never fails + } finally { + await t.close(); + } + } finally { + testDb.close(); + } + }); +}); + +/** + * The reviewer's exact repro (`task-4-review-shape.md`, Important finding + * 1): before the fix, `em.create(BudgetItems, …)` and + * `em.create(IdempotencyKeys, …)` both threw `SqliteError: no such column: + * NaN` at `flush()` — Kysely rendered the `NaN` class-field initialiser as a + * bare SQL identifier. Neither probe names the broken columns + * (`persons`/`days`/`place_id`/`reservation_id` on `BudgetItems`, + * `created_at` on `IdempotencyKeys`) — the whole point is that a caller + * should never have to. + */ +describe('insert probes (task-4-review-shape.md, the reviewer\'s exact repro)', () => { + const testDb = createSnapshotTestDb(); + let t: TestOrm; + + beforeAll(async () => { + t = await createTestOrm(testDb); + }); + beforeEach(() => { + resetTestDb(testDb); + t.clear(); + }); + afterAll(async () => { + await t.close(); + testDb.close(); + }); + + it('INSERT-PROBE-001: em.create(BudgetItems, { trip, name, total_price, exchange_rate }) + flush succeeds', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + + const created = t.em.create(BudgetItems, { + trip: trip.id, + name: 'Dinner', + total_price: 4200, + exchange_rate: 1, + }); + await t.em.persist(created).flush(); + + const row = testDb.prepare('SELECT persons, days, place_id, reservation_id FROM budget_items WHERE id = ?').get(created.id); + expect(row).toEqual({ persons: null, days: null, place_id: null, reservation_id: null }); + }); + + it('INSERT-PROBE-002: em.create(IdempotencyKeys, { key, user, method, path, status_code, response_body }) + flush succeeds', async () => { + const { user } = createUser(testDb); + + const created = t.em.create(IdempotencyKeys, { + key: 'idem-key-probe-1', + user: user.id, + method: 'POST', + path: '/api/trips', + status_code: 201, + response_body: '{}', + }); + await t.em.persist(created).flush(); + + const row = testDb + .prepare('SELECT created_at FROM idempotency_keys WHERE key = ? AND user_id = ? AND method = ? AND path = ?') + .get('idem-key-probe-1', user.id, 'POST', '/api/trips') as { created_at: number }; + // The DB's own defaultRaw (strftime('%s','now')) filled it in — never NaN, never null. + expect(row.created_at).toEqual(expect.any(Number)); + expect(Number.isNaN(row.created_at)).toBe(false); + }); +}); diff --git a/server/tests/unit/db/entity-schema-parity.test.ts b/server/tests/unit/db/entity-schema-parity.test.ts index 20487eb61..b25b8d1b7 100644 --- a/server/tests/unit/db/entity-schema-parity.test.ts +++ b/server/tests/unit/db/entity-schema-parity.test.ts @@ -419,16 +419,28 @@ describe('entity ↔ migrated-schema parity', () => { * folds a `p.boolean()` property's literal `true`/`false` default onto * SQLite's `1`/`0` (SQLite has no boolean storage class — see * `BOOLEAN_COLUMNS` in `scripts/generate-entities.ts`). Used by PARITY-005. + * + * M1 (task-4-review-gates.md): only the SQL keywords/functions above are + * folded case-insensitively — a quoted string literal's PAYLOAD is data, not + * a keyword, and is compared verbatim, case included. A blanket + * `s.toLowerCase()` on the whole string (the previous shape) silently + * equated `'#10b981'` and `'#10B981'`, proved by the gate reviewer's mutation + * (d2): flipping `Tags.color`'s class-field default's case left every test + * green. The entity side and the db side aren't even symmetric about + * quoting to begin with (`prop.default` arrives as a bare JS string, e.g. + * `#10b981`; `column.dflt_value` arrives SQL-quoted, `'#10b981'`) — quotes + * are stripped from whichever side has them, then BOTH sides are compared + * verbatim, never lowercased, unless they match a known keyword. */ function normaliseDefault(value: string | null): string | null { if (value == null) return null; let s = value.trim(); if (s.startsWith('(') && s.endsWith(')')) s = s.slice(1, -1).trim(); - if (s.startsWith("'") && s.endsWith("'")) s = s.slice(1, -1); - s = s.toLowerCase(); - if (s === "datetime('now')" || s === 'current_timestamp') s = '__now__'; - if (s === 'true') s = '1'; - if (s === 'false') s = '0'; + if (s.startsWith("'") && s.endsWith("'")) s = s.slice(1, -1); // SQL quoting stripped; payload kept verbatim below + const lower = s.toLowerCase(); + if (lower === "datetime('now')" || lower === 'current_timestamp') return '__now__'; + if (lower === 'true') return '1'; + if (lower === 'false') return '0'; return s; } @@ -453,6 +465,39 @@ describe('normaliseColumnType', () => { }); }); +describe('normaliseDefault', () => { + it('DEFAULT-NORM-001: a case-only difference in a quoted string literal payload is a real mismatch (M1)', () => { + expect(normaliseDefault("'#10b981'")).not.toBe(normaliseDefault("'#10B981'")); + }); + + it('DEFAULT-NORM-002: an identical quoted string literal payload matches, case preserved', () => { + expect(normaliseDefault("'#10b981'")).toBe(normaliseDefault("'#10b981'")); + expect(normaliseDefault("'#10b981'")).toBe('#10b981'); + }); + + it('DEFAULT-NORM-003: an unquoted default (as PARITY-005 passes the entity side, String(prop.default)) compares verbatim too', () => { + expect(normaliseDefault('#10B981')).toBe('#10B981'); + expect(normaliseDefault('#10B981')).not.toBe(normaliseDefault("'#10b981'")); + }); + + it('DEFAULT-NORM-004: SQL keyword/function spellings still fold case-insensitively', () => { + expect(normaliseDefault('CURRENT_TIMESTAMP')).toBe('__now__'); + expect(normaliseDefault('current_timestamp')).toBe('__now__'); + expect(normaliseDefault("datetime('now')")).toBe('__now__'); + expect(normaliseDefault("DATETIME('NOW')")).toBe('__now__'); + expect(normaliseDefault('TRUE')).toBe('1'); + expect(normaliseDefault('FALSE')).toBe('0'); + }); + + it('DEFAULT-NORM-005: a wrapping paren pair around a non-literal expression is stripped once', () => { + expect(normaliseDefault("(strftime('%s','now'))")).toBe("strftime('%s','now')"); + }); + + it('DEFAULT-NORM-006: null passes through as null', () => { + expect(normaliseDefault(null)).toBeNull(); + }); +}); + describe('columnTypesEqual', () => { it('TYPE-004: accepts datetime on one side and text on the other, either direction', () => { expect(columnTypesEqual('datetime', 'text')).toBe(true); diff --git a/server/tests/unit/db/generate-entities.test.ts b/server/tests/unit/db/generate-entities.test.ts index 5c12a447b..2e761faa8 100644 --- a/server/tests/unit/db/generate-entities.test.ts +++ b/server/tests/unit/db/generate-entities.test.ts @@ -12,6 +12,7 @@ import { RULE1_fixUnknownScalarTypes, RULE1b_markJsonColumns, RULE1c_markBooleanColumns, + RULE1d_fixNaNNumericDefaults, RULE2_datetimeToDbTimestampType, RULE3_integerPkAutoincrement, RULE4_hideAllRelations, @@ -21,6 +22,8 @@ import { RULE8_bindRepositories, RULE_normalizeLiteralDefaults, applyTextPasses, + checkEntities, + dumpEntities, fixDbTimestampClassFieldTypes, fixJsonClassFieldTypes, generateEntities, @@ -35,6 +38,7 @@ import { } from '../../../scripts/generate-entities'; const ENTITIES_DIR = path.join(__dirname, '../../../src/db/entities'); +const REPOSITORIES_DIR = path.join(__dirname, '../../../src/db/repositories'); /** * A minimal, correctly-typed `EntityProperty` fixture. Every rule under test @@ -169,6 +173,56 @@ describe('RULE1c_markBooleanColumns', () => { }); }); +describe('RULE1d_fixNaNNumericDefaults', () => { + /** + * `task-4-review-shape.md` Important finding 1: `@mikro-orm/sql`'s own + * introspection computes `prop.default` for a numeric column as + * `+prop.defaultRaw`, which is `NaN` whenever `defaultRaw` is the literal + * text `'NULL'` (a nullable column with an explicit `DEFAULT NULL`) or a + * non-literal SQL expression (`(strftime('%s','now'))`) — the exact repro + * the reviewer ran against the real schema (`BudgetItems.persons`/`.days`, + * `IdempotencyKeys.created_at`, `BudgetItems.place_id`/`.reservation_id`, + * `PackingBags.user_id`). + */ + it("RULE1D-001: defaultRaw 'NULL' with a NaN default clears the initialiser (never renders = null)", () => { + const p = fixtureProp({ name: 'persons', type: 'integer', nullable: true, defaultRaw: 'NULL', default: NaN }); + const meta = fixtureMeta('BudgetItems', 'budget_items', [p]); + RULE1d_fixNaNNumericDefaults([meta]); + expect(p.default).toBeUndefined(); + expect(p.defaultRaw).toBe('NULL'); // untouched — the schema's .defaultRaw('NULL') is correct and stays + }); + + it('RULE1D-002: a non-literal expression default with a NaN default clears the initialiser', () => { + const p = fixtureProp({ name: 'created_at', type: 'integer', nullable: false, defaultRaw: "(strftime('%s','now'))", default: NaN }); + const meta = fixtureMeta('IdempotencyKeys', 'idempotency_keys', [p]); + RULE1d_fixNaNNumericDefaults([meta]); + expect(p.default).toBeUndefined(); + expect(p.defaultRaw).toBe("(strftime('%s','now'))"); // untouched + }); + + it("RULE1D-003: a genuine finite numeric literal default ('0') is left completely untouched", () => { + const p = fixtureProp({ name: 'sort_order', type: 'integer', nullable: false, defaultRaw: '0', default: 0 }); + const meta = fixtureMeta('X', 'x', [p]); + RULE1d_fixNaNNumericDefaults([meta]); + expect(p.default).toBe(0); + expect(p.defaultRaw).toBe('0'); + }); + + it('RULE1D-004: a property whose default is not NaN (e.g. a text column default(raw) NULL, default null) is untouched', () => { + const p = fixtureProp({ name: 'note', type: 'text', nullable: true, defaultRaw: 'NULL', default: null }); + const meta = fixtureMeta('X', 'x', [p]); + RULE1d_fixNaNNumericDefaults([meta]); + expect(p.default).toBeNull(); + }); + + it('RULE1D-005: a property with no defaultRaw at all is untouched', () => { + const p = fixtureProp({ name: 'title', type: 'text' }); + const meta = fixtureMeta('X', 'x', [p]); + RULE1d_fixNaNNumericDefaults([meta]); + expect(p.default).toBeUndefined(); + }); +}); + describe('RULE2_datetimeToDbTimestampType', () => { it('RULE2-001: p.datetime() -> p.type(DbTimestampType), fixup recorded', () => { const p = fixtureProp({ name: 'created_at', type: 'datetime', nullable: true, defaultRaw: 'CURRENT_TIMESTAMP' }); @@ -637,6 +691,92 @@ describe('regenerateEntitiesIndex', () => { }); }); +/** + * `--check` (task-4-review-gates.md, I1): closes the gap the gate reviewer + * proved — losing `.hidden()` on a relation (or any other rule regression) + * passes every other suite for 347 of 396 relation lines. These tests exercise + * the real generated `files` (from `generateEntities()`) against a SCRATCH + * COPY of the committed `src/db/entities/` — never the real directory itself, + * so a failing assertion here can never leave the working tree dirty. + */ +describe('checkEntities', () => { + it( + 'CHECK-001: a hand-mutated entity in the scratch copy is reported by name, exit-worthy', + async () => { + const { files } = await generateEntities(); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gen-entities-check-')); + try { + fs.cpSync(ENTITIES_DIR, dir, { recursive: true }); + // Same shape as VALIDATE-004's mutation (MUT-B): drop a ONE_TO_MANY relation's .hidden(). + const daysPath = path.join(dir, 'Days.entity.ts'); + const original = fs.readFileSync(daysPath, 'utf8'); + const mutated = original.replace( + "day_accommodations_collection: () => p.oneToMany(DayAccommodations).mappedBy('startDay').hidden(),", + "day_accommodations_collection: () => p.oneToMany(DayAccommodations).mappedBy('startDay'),", + ); + expect(mutated, 'the mutation string was not found in the committed file — update it to match the current shape').not.toBe( + original, + ); + fs.writeFileSync(daysPath, mutated); + + const report = checkEntities(dir, REPOSITORIES_DIR, files); + expect(report.differingFiles).toEqual(['Days.entity.ts']); + expect(report.missingRepositories).toEqual([]); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }, + 30_000, + ); + + it( + 'CHECK-002: an untouched copy of the committed tree reports no differences and no missing repositories', + async () => { + const { files } = await generateEntities(); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gen-entities-check-')); + try { + fs.cpSync(ENTITIES_DIR, dir, { recursive: true }); + const report = checkEntities(dir, REPOSITORIES_DIR, files); + expect(report.differingFiles).toEqual([]); + expect(report.missingRepositories).toEqual([]); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }, + 30_000, + ); + + it('CHECK-003: a missing entity file is reported', async () => { + const files = new Map([['X.entity.ts', 'content']]); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gen-entities-check-')); + const repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gen-entities-check-repos-')); + try { + const report = checkEntities(dir, repoDir, files); + expect(report.differingFiles).toEqual(['X.entity.ts', 'index.ts']); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + fs.rmSync(repoDir, { recursive: true, force: true }); + } + }); + + it('CHECK-004: a missing repository file is reported without being created (never writes)', async () => { + const files = new Map([['X.entity.ts', 'content']]); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gen-entities-check-')); + const repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gen-entities-check-repos-')); + try { + dumpEntities(dir, files); + fs.writeFileSync(path.join(dir, 'index.ts'), regenerateEntitiesIndex(dir)); + const report = checkEntities(dir, repoDir, files); + expect(report.differingFiles).toEqual([]); + expect(report.missingRepositories).toEqual([path.join(repoDir, 'X.repository.ts')]); + expect(fs.existsSync(path.join(repoDir, 'X.repository.ts'))).toBe(false); // never writes + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + fs.rmSync(repoDir, { recursive: true, force: true }); + } + }); +}); + describe('assertFilesGenerated', () => { it('GUARD-001: throws when zero files were generated (I5)', () => { expect(() => assertFilesGenerated(new Map())).toThrow(/zero entity files/); @@ -850,6 +990,28 @@ describe('PENDING_ENTITIES ratchet (I7)', () => { } expect(failures).toEqual([]); }); + + /** + * M2 (task-4-review-gates.md): the mirror arm ENTITIES_STILL_MISSING has + * (PARITY-008) and this list did not — a listed class name that names + * nothing the generator actually produces is a stale entry, not a real + * pending tolerance, and `stripPendingEntityRelations` would silently no-op + * on it forever. Every name in `PENDING_ENTITIES` must appear somewhere in + * `generateEntities()`'s own output (as a relation type reference in + * another entity's file, the normal way a class with no entity of its own + * yet still shows up — see the class doc comment above). + */ + it("PENDING-ENTITIES-002: stays accurate — every one of them must appear in generateEntities()'s output (M2)", async () => { + const { files } = await generateEntities(); + const allGeneratedSource = [...files.values()].join('\n'); + const failures: string[] = []; + for (const className of PENDING_ENTITIES) { + if (!new RegExp(`\\b${className}\\b`).test(allGeneratedSource)) { + failures.push(`${className}: listed in PENDING_ENTITIES but does not appear anywhere in generateEntities()'s output`); + } + } + expect(failures).toEqual([]); + }, 30_000); }); describe('generateEntities — migrations path is anchored to SERVER_ROOT, not cwd (I5)', () => { diff --git a/server/vitest.config.ts b/server/vitest.config.ts index 8a7e68fb4..bcb7c71b7 100644 --- a/server/vitest.config.ts +++ b/server/vitest.config.ts @@ -172,7 +172,7 @@ export default defineConfig({ // src/demo/** is deliberately absent: it measures 0%, and a floor of // zero asserts nothing. It needs tests before it needs a threshold. 'src/app-config/**/*.ts': { statements: 99, branches: 95, functions: 99, lines: 99 }, - 'src/db/**/*.ts': { statements: 73, branches: 38, functions: 59, lines: 80 }, + 'src/db/**/*.ts': { statements: 84, branches: 42, functions: 95, lines: 87 }, 'src/db/dialect/**/*.ts': { statements: 99, branches: 99, functions: 99, lines: 99 }, 'src/db/repositories/**/*.ts': { statements: 99, branches: 99, functions: 99, lines: 99 }, 'src/db/types/**/*.ts': { statements: 93, branches: 99, functions: 99, lines: 93 },