fix(db): generator emits the referenced column for non-PK foreign keys; OAuth client joins through the relation; FK parity un-skipped

server/tests/unit/db/entity-schema-parity.test.ts

@@ -303,34 +303,83 @@ describe('entity ↔ migrated-schema parity', () => {
   });
 
   /**
-   * FK targets/deleteRule and index names, against `PRAGMA foreign_key_list`
-   * and `PRAGMA index_list` — the plan's second, allow-listed check
-   * (Task 0, Step 3). Task 0 found deleteRule disagreeing on 11 of 180
-   * owning relations; Task 3's rewrite fixed one of those eleven as a side
-   * effect (`school_holiday_regions.country` now has a real, explicit
-   * `joinColumn`, so it reports a genuine FK row) and re-ran this check
-   * against the other 124 rewritten/added entities, composite-PK relations
-   * included (`packing_item_contributors`, `vacay_user_settings`,
-   * `document_connections`, `trip_document_links` — their `.entity.ts`
-   * source never spells `.deleteRule(...)` on those relations, but MikroORM
-   * discovery still resolves the correct rule at runtime, confirmed by
-   * running this check, so they are not drift): the count is now exactly 10,
-   * unchanged in substance from Task 0's list minus the one now-fixed row
-   * (`trip_members.invited_by` entity=set null vs db=no action,
-   * `roadtrip_day_tracks.day_id` entity=set null vs db=cascade,
-   * `reservation_travelers.reservation_id`/`.user_id` and
+   * FK targets, against `PRAGMA foreign_key_list` — the plan's second,
+   * allow-listed check (Task 0, Step 3), the REFERENCED-COLUMN half. Task 0
+   * left the whole check (referenced column + deleteRule + index names)
+   * `it.skip`'d and report-only. This half is un-skipped as of Plan 3b
+   * interlude A: `RULE11_referencedColumns` (`scripts/generate-entities.ts`)
+   * closed the one gap that made it fail — `OauthTokens.client`/
+   * `OauthConsents.client` targeted `oauth_clients.id` (the generator's
+   * default assumption for an owning to-one relation) instead of the schema's
+   * real FK target, the UNIQUE natural key `oauth_clients.client_id`
+   * (`Migration20200101012500_oauth_2.ts`). Every other owning relation in
+   * the schema already agreed with `PRAGMA foreign_key_list`'s `to` column,
+   * `school_holiday_regions.country -> school_holiday_countries.code`
+   * included (`code` IS that table's primary key, so the generator's default
+   * assumption was already correct there — unchanged by Rule 11, confirmed
+   * by this same test). The deleteRule half stays skipped — see
+   * `PARITY-009b` immediately below for why and by how many rows.
+   */
+  it('PARITY-009: FK referenced columns agree with PRAGMA foreign_key_list', () => {
+    const failures: string[] = [];
+    for (const meta of tableBackedMetas()) {
+      const table = meta.tableName;
+      const fkRows = testDb.prepare(`PRAGMA foreign_key_list("${table}")`).all() as {
+        from: string;
+        table: string;
+        to: string;
+      }[];
+      const fkByColumn = new Map(fkRows.map((row) => [row.from, row]));
+      for (const prop of meta.props) {
+        const isOwningToOne =
+          prop.kind === ReferenceKind.MANY_TO_ONE || (prop.kind === ReferenceKind.ONE_TO_ONE && prop.owner);
+        if (!isOwningToOne) continue;
+        const fieldName = prop.fieldNames[0];
+        const fk = fkByColumn.get(fieldName);
+        if (!fk) {
+          failures.push(`${table}.${fieldName}: entity relation has no matching db foreign key`);
+          continue;
+        }
+        const entityReferencedColumn = prop.referencedColumnNames?.[0];
+        if (entityReferencedColumn !== fk.to) {
+          failures.push(`${table}.${fieldName}: referencedColumn entity=${String(entityReferencedColumn)} vs db=${fk.to}`);
+        }
+      }
+    }
+    expect(failures).toEqual([]);
+  });
+
+  /**
+   * The deleteRule half of the same check (Task 0, Step 3). Task 0 found
+   * deleteRule disagreeing on 11 of 180 owning relations; Task 3's rewrite
+   * fixed one of those eleven as a side effect (`school_holiday_regions.
+   * country` now has a real, explicit `joinColumn`, so it reports a genuine
+   * FK row) and re-ran this check against the other 124 rewritten/added
+   * entities, composite-PK relations included (`packing_item_contributors`,
+   * `vacay_user_settings`, `document_connections`, `trip_document_links` —
+   * their `.entity.ts` source never spells `.deleteRule(...)` on those
+   * relations, but MikroORM discovery still resolves the correct rule at
+   * runtime, confirmed by running this check, so they are not drift): the
+   * count was 10. Plan 3b interlude A's `RULE11_referencedColumns` only
+   * touches the referenced COLUMN, never `deleteRule` (`OauthTokens.client`/
+   * `OauthConsents.client` already agreed on `deleteRule('cascade')` —
+   * unaffected either way), so re-running this exact query still finds
+   * **10** disagreements, unchanged: `trip_members.invited_by` entity=set
+   * null vs db=no action, `roadtrip_day_tracks.day_id` entity=set null vs
+   * db=cascade, `reservation_travelers.reservation_id`/`.user_id` and
    * `assignment_participants.assignment_id`/`.user_id` entity=no action vs
    * db=cascade, `oauth_tokens.parent_token_id` entity=set null vs db=no
    * action, `journey_contributors.user_id` entity=cascade vs db=no action,
    * `budget_settlements.created_by_user_id`/`budget_items.paid_by_user_id`
-   * entity=set null vs db=no action). None of these are in Task 0's or
-   * Task 3's scope; an allow-list would need 10 one-line entries for drift
-   * neither task was asked to fix, and index-name parity looked clean on
-   * inspection but was not exhaustively checked. Left skipped, with the
-   * findings above, for Plan 4 to turn into either fixes or a real
-   * allow-list.
+   * entity=set null vs db=no action. None of these are in this task's scope
+   * (or Task 0's/Task 3's); an allow-list would need 10 one-line entries for
+   * drift no task so far was asked to fix. Index-name parity looked clean on
+   * inspection but was not exhaustively checked either, and stays out of
+   * scope here too. Left skipped, with the count pinned above (Plan 4 owns
+   * turning this into either fixes or a real allow-list — the number to
+   * reconcile against is exactly 10, not "some").
    */
-  it.skip('PARITY-009 (report-only, deferred to Plan 4): FK deleteRule and index names agree', () => {
+  it.skip('PARITY-009b (report-only, deferred to Plan 4): FK deleteRule agrees — 10 known disagreements, see doc comment', () => {
     const failures: string[] = [];
     for (const meta of tableBackedMetas()) {
       const table = meta.tableName;

server/tests/unit/db/generate-entities.test.ts

@@ -13,6 +13,7 @@ import {
   JoinColumnFixup,
   JsonColumnFixup,
   KNOWN_DIFFS,
+  ReferencedColumnsFixup,
   RepositoryTypeMarkerFixup,
   RULE1_fixUnknownScalarTypes,
   RULE1b_markJsonColumns,
@@ -27,6 +28,7 @@ import {
   RULE8_bindRepositories,
   RULE9_addImplicitUniqueConstraints,
   RULE10_repositoryTypeMarker,
+  RULE11_referencedColumns,
   RULE_normalizeLiteralDefaults,
   applyTextPasses,
   checkEntities,
@@ -38,6 +40,7 @@ import {
   injectJsonInterfaces,
   injectJsonTypeParams,
   injectMissingDefaults,
+  injectReferencedColumns,
   injectRepositoryTypeMarker,
   regenerateEntitiesIndex,
   stripHiddenTypeAnnotation,
@@ -437,6 +440,58 @@ describe('RULE10_repositoryTypeMarker', () => {
   });
 });
 
+describe('RULE11_referencedColumns', () => {
+  it('RULE11-001: an FK targeting its target\'s own single-column PK gets no fixup (the common case)', () => {
+    const rel = fixtureProp({ name: 'user', kind: ReferenceKind.MANY_TO_ONE, fieldNames: ['user_id'], type: 'Users', referencedColumnNames: ['id'] });
+    const owner = fixtureMeta('OauthTokens', 'oauth_tokens', [rel]);
+    const target = fixtureMeta('Users', 'users', [fixtureProp({ name: 'id', primary: true })]);
+    const fixups = RULE11_referencedColumns([owner, target]);
+    expect(fixups).toEqual([]);
+  });
+
+  it('RULE11-002: an FK targeting a non-PK column (the real oauth_tokens.client_id -> oauth_clients.client_id case) gets a fixup', () => {
+    const rel = fixtureProp({ name: 'client', kind: ReferenceKind.MANY_TO_ONE, fieldNames: ['client_id'], type: 'OauthClients', referencedColumnNames: ['client_id'] });
+    const owner = fixtureMeta('OauthTokens', 'oauth_tokens', [rel]);
+    const target = fixtureMeta('OauthClients', 'oauth_clients', [fixtureProp({ name: 'id', primary: true })]);
+    const fixups = RULE11_referencedColumns([owner, target]);
+    expect(fixups).toEqual<ReferencedColumnsFixup[]>([{ className: 'OauthTokens', propName: 'client', referencedColumnNames: ['client_id'] }]);
+  });
+
+  it('RULE11-003: a text PK target whose FK genuinely references that PK (school_holiday_regions.country -> school_holiday_countries.code) gets no fixup', () => {
+    const rel = fixtureProp({ name: 'countryRef', kind: ReferenceKind.MANY_TO_ONE, fieldNames: ['country'], type: 'SchoolHolidayCountries', referencedColumnNames: ['code'] });
+    const owner = fixtureMeta('SchoolHolidayRegions', 'school_holiday_regions', [rel]);
+    const target = fixtureMeta('SchoolHolidayCountries', 'school_holiday_countries', [fixtureProp({ name: 'code', primary: true })]);
+    const fixups = RULE11_referencedColumns([owner, target]);
+    expect(fixups).toEqual([]);
+  });
+
+  it('RULE11-004: an unresolved relation target (no matching metadata) is skipped, not thrown or flagged', () => {
+    const rel = fixtureProp({ name: 'something', kind: ReferenceKind.MANY_TO_ONE, fieldNames: ['something_id'], type: 'Nowhere', referencedColumnNames: ['id'] });
+    const owner = fixtureMeta('X', 'x', [rel]);
+    expect(() => RULE11_referencedColumns([owner])).not.toThrow();
+    expect(RULE11_referencedColumns([owner])).toEqual([]);
+  });
+
+  it('RULE11-005: an inverse relation (oneToMany) is never a candidate — only an owning to-one is checked', () => {
+    const inverse = fixtureProp({ name: 'oauth_tokens_collection', kind: ReferenceKind.ONE_TO_MANY, type: 'OauthTokens', mappedBy: 'client', referencedColumnNames: ['id'] });
+    const owner = fixtureMeta('OauthClients', 'oauth_clients', [inverse]);
+    const target = fixtureMeta('OauthTokens', 'oauth_tokens', [fixtureProp({ name: 'id', primary: true })]);
+    const fixups = RULE11_referencedColumns([owner, target]);
+    expect(fixups).toEqual([]);
+  });
+
+  it('RULE11-006: a composite-PK target compares the full column list, in order', () => {
+    const rel = fixtureProp({ name: 'setting', kind: ReferenceKind.MANY_TO_ONE, fieldNames: ['a', 'b'], type: 'Composite', referencedColumnNames: ['b', 'a'] });
+    const owner = fixtureMeta('X', 'x', [rel]);
+    const target = fixtureMeta('Composite', 'composite', [
+      fixtureProp({ name: 'a', primary: true }),
+      fixtureProp({ name: 'b', primary: true }),
+    ]);
+    const fixups = RULE11_referencedColumns([owner, target]);
+    expect(fixups).toEqual<ReferencedColumnsFixup[]>([{ className: 'X', propName: 'setting', referencedColumnNames: ['b', 'a'] }]);
+  });
+});
+
 describe('RULE9_addImplicitUniqueConstraints', () => {
   it('RULE9-001: a table with a matching implicit-unique entry gets a uniques: block referencing the columns as properties — a plain (non-FK) column keeps its own name', () => {
     const userId = fixtureProp({ name: 'user_id', primary: false });
@@ -806,6 +861,51 @@ describe('injectRepositoryTypeMarker', () => {
   });
 });
 
+describe('injectReferencedColumns', () => {
+  it('TEXT-REFCOL-001: appends .referencedColumnNames(<col>) before the trailing comma (the real client_id shape)', () => {
+    const source = "    client: () => p.manyToOne(OauthClients).ref().name('client_id').deleteRule('cascade').hidden(),\n";
+    const out = injectReferencedColumns(source, [{ className: 'OauthTokens', propName: 'client', referencedColumnNames: ['client_id'] }]);
+    expect(out).toBe("    client: () => p.manyToOne(OauthClients).ref().name('client_id').deleteRule('cascade').hidden().referencedColumnNames('client_id'),\n");
+  });
+
+  it('TEXT-REFCOL-002: multiple referenced columns render as bare comma-separated quoted strings, no array brackets (rest-args API)', () => {
+    const source = '    setting: () => p.manyToOne(Composite).ref(),\n';
+    const out = injectReferencedColumns(source, [{ className: 'X', propName: 'setting', referencedColumnNames: ['b', 'a'] }]);
+    expect(out).toContain(".referencedColumnNames('b', 'a')");
+    expect(out).not.toContain('[');
+  });
+
+  it('TEXT-REFCOL-003: a line already carrying an explicit .referencedColumnNames( call is left untouched (legitimate no-op)', () => {
+    const source = "    client: () => p.manyToOne(OauthClients).ref().referencedColumnNames('client_id'),\n";
+    const out = injectReferencedColumns(source, [{ className: 'X', propName: 'client', referencedColumnNames: ['client_id'] }]);
+    expect(out).toBe(source);
+  });
+
+  it('TEXT-REFCOL-004: appends before a trailing comma even with a line comment after it', () => {
+    const source = '    client: () => p.manyToOne(Y).ref(), // pins client\n';
+    const out = injectReferencedColumns(source, [{ className: 'X', propName: 'client', referencedColumnNames: ['client_id'] }]);
+    expect(out).toBe("    client: () => p.manyToOne(Y).ref().referencedColumnNames('client_id'), // pins client\n");
+  });
+
+  it('TEXT-REFCOL-005: throws, naming the class and property, if the property line is not found', () => {
+    expect(() => injectReferencedColumns('', [{ className: 'X', propName: 'client', referencedColumnNames: ['client_id'] }])).toThrow(
+      /could not find the property line for "X\.client"/,
+    );
+  });
+
+  it('TEXT-REFCOL-006: throws if the property line has no trailing comma to anchor on', () => {
+    const source = '    client: () => p.manyToOne(Y).ref()';
+    expect(() => injectReferencedColumns(source, [{ className: 'X', propName: 'client', referencedColumnNames: ['client_id'] }])).toThrow(
+      /could not append/,
+    );
+  });
+
+  it('TEXT-REFCOL-007: an empty fixups list is a no-op', () => {
+    const source = "    client: () => p.manyToOne(OauthClients).ref().name('client_id'),\n";
+    expect(injectReferencedColumns(source, [])).toBe(source);
+  });
+});
+
 describe('applyTextPasses', () => {
   it('TEXT-ALL-001: composes every pass without one undoing another', () => {
     const source =
@@ -821,8 +921,10 @@ describe('applyTextPasses', () => {
       jsonColumns: [],
       retypedScalars: [],
       repositoryMarkers: [{ className: 'X', repositoryClassName: 'XRepository' }],
+      referencedColumns: [{ className: 'X', propName: 'countryRef', referencedColumnNames: ['code'] }],
     });
     expect(out).toContain(".joinColumn('country')");
+    expect(out).toContain(".referencedColumnNames('code')");
     expect(out).not.toContain('& Hidden');
     expect(out).toContain('export class X {\n  [EntityRepositoryType]?: XRepository;\n  countryRef!: Ref<Y>;\n');
   });

server/tests/unit/db/repositories/OauthTokens.repository.test.ts

@@ -344,7 +344,7 @@ describe('OauthTokensRepository', () => {
     });
   });
 
-  describe('listActiveByUser (OA26) — Kysely join on the real oauth_clients.client_id column', () => {
+  describe('listActiveByUser (OA26) — QueryBuilder join on the real oauth_clients.client_id column', () => {
     it('OAUTHTOKREPO-022: lists only this user\'s non-revoked, non-expired-refresh sessions, newest first, with the client name joined in', async () => {
       const { user } = createUser(testDb);
       const { user: other } = createUser(testDb);
@@ -434,6 +434,57 @@ describe('OauthTokensRepository', () => {
     });
   });
 
+  describe('client relation join (Plan 3b interlude A — RULE11_referencedColumns)', () => {
+    it('OAUTHTOKREPO-030: a QueryBuilder join on the client relation resolves the right client, even though oauth_clients.id !== client_id', async () => {
+      const { user } = createUser(testDb);
+      // seedClient's own `id` ("row-<clientId>") is already never equal to
+      // `client_id` — this test additionally asserts the two literally
+      // differ, so a future fixture change can't silently make the
+      // distinction disappear and hide a regression.
+      seedClient(user.id, 'proto-30', 'Client Thirty');
+      const clientRow = testDb.prepare('SELECT id, client_id FROM oauth_clients WHERE client_id = ?').get('proto-30') as { id: string; client_id: string };
+      expect(clientRow.id).not.toBe(clientRow.client_id);
+      const id = seedToken({ clientId: 'proto-30', userId: user.id });
+
+      const em = t.orm.em.fork();
+      const qb = em.getRepository(OauthTokens).qb('ot').innerJoin('ot.client', 'oc').select(['ot.id', 'oc.name as client_name']).where({ 'ot.id': id });
+      expect(qb.getFormattedQuery()).toContain('inner join `oauth_clients` as `oc` on `ot`.`client_id` = `oc`.`client_id`');
+      const row = await qb.execute<{ id: number; client_name: string }>('get', false);
+      expect(row).toEqual({ id, client_name: 'Client Thirty' });
+    });
+
+    it('OAUTHTOKREPO-031: a DECOY client whose row id equals the real client_id — regression-proofs the join against silently falling back to oauth_clients.id (task-4-review.md, "For Task 5 / interlude A")', async () => {
+      const { user } = createUser(testDb);
+      seedClient(user.id, 'proto-31-real', 'Real Client');
+      const realClientRow = testDb
+        .prepare('SELECT id, client_id FROM oauth_clients WHERE client_id = ?')
+        .get('proto-31-real') as { id: string; client_id: string };
+      // The decoy's own PRIMARY KEY (`id`) is deliberately set to the REAL
+      // client's `client_id` — a wrong join (`oc.id = ot.client_id`, the pre-
+      // Rule-11 defect) would match THIS row instead of the real one, and
+      // return the decoy's name, not silently return nothing. A regression
+      // to the wrong column is caught by a WRONG answer, not just an absent
+      // one — the strongest form of this proof.
+      testDb.prepare(
+        `INSERT INTO oauth_clients (id, user_id, name, client_id, client_secret_hash, redirect_uris, allowed_scopes) VALUES (?, ?, ?, ?, ?, '[]', '[]')`,
+      ).run(realClientRow.client_id, user.id, 'Decoy Client', 'proto-31-decoy', 'hash');
+
+      const id = seedToken({ clientId: 'proto-31-real', userId: user.id });
+
+      const em = t.orm.em.fork();
+      const qb = em.getRepository(OauthTokens).qb('ot').innerJoin('ot.client', 'oc').select(['ot.id', 'oc.name as client_name']).where({ 'ot.id': id });
+      const row = await qb.execute<{ id: number; client_name: string }>('get', false);
+      expect(row).toEqual({ id, client_name: 'Real Client' });
+
+      // Same proof through the actual repository methods (OA26/OA31), not
+      // just a hand-rolled QueryBuilder query.
+      const active = await tokens.listActiveByUser(user.id);
+      expect(active.map((r) => r.client_name)).toEqual(['Real Client']);
+      const allActive = await tokens.listAllActiveWithClientAndUser();
+      expect(allActive.map((r) => r.client_name)).toEqual(['Real Client']);
+    });
+  });
+
   describe('identity-map write-back regression (program RULING, D-shape)', () => {
     it('OAUTHTOKREPO-029: findByRefreshTokenHash (projection A) then findParent (projection B) then revokeById inside uow.transactional — the write survives', async () => {
       const { user } = createUser(testDb);

server/tests/unit/nest/common/row-id.test.ts

@@ -7,11 +7,11 @@ describe('toRowId', () => {
     expect(toRowId('abc')).toBeNull();
   });
 
-  it('ROWID-002: rejects a hex-shaped string — Number() would accept it, SQLite affinity never would', () => {
+  it('ROWID-002: rejects a hex-shaped string — Number() would accept it, SQLite affinity never would (verified against better-sqlite3)', () => {
     expect(toRowId('0x10')).toBeNull();
   });
 
-  it('ROWID-003: rejects an exponent-shaped string — Number() would accept it, SQLite affinity never would', () => {
+  it('ROWID-003: rejects an exponent-shaped string ("1e3") — a DELIBERATE, ACCEPTED deviation: SQLite\'s own numeric-affinity comparison DOES match this against integer 1000 (verified against better-sqlite3, not assumed — the row-id.ts docstring previously claimed the opposite), but no id this guard ever sees originates that way (every id is a canonical decimal string from our own client), so this fails closed to the route\'s legacy not-found result instead of widening the guard to match SQLite\'s affinity exactly', () => {
     expect(toRowId('1e3')).toBeNull();
   });
 
@@ -19,14 +19,22 @@ describe('toRowId', () => {
     expect(toRowId('-1')).toBeNull();
   });
 
-  it('ROWID-005: rejects a leading space', () => {
+  it('ROWID-005: rejects a leading space (" 1") — same deliberate deviation as ROWID-003: SQLite\'s affinity conversion ignores leading whitespace and matches integer 1 (verified against better-sqlite3)', () => {
     expect(toRowId(' 1')).toBeNull();
   });
 
-  it('ROWID-006: rejects a decimal point', () => {
+  it('ROWID-005B: rejects a trailing space ("1 ") — same deliberate deviation: SQLite\'s affinity conversion ignores trailing whitespace too and matches integer 1 (verified against better-sqlite3)', () => {
+    expect(toRowId('1 ')).toBeNull();
+  });
+
+  it('ROWID-006: rejects a decimal point ("1.0") — same deliberate deviation: SQLite\'s affinity conversion parses this as REAL 1.0, which compares equal to INTEGER 1 (verified against better-sqlite3)', () => {
     expect(toRowId('1.0')).toBeNull();
   });
 
+  it('ROWID-006B: rejects a leading plus sign ("+1") — same deliberate deviation: SQLite\'s affinity conversion accepts a leading sign and matches integer 1 (verified against better-sqlite3)', () => {
+    expect(toRowId('+1')).toBeNull();
+  });
+
   it('ROWID-007: rejects an empty string', () => {
     expect(toRowId('')).toBeNull();
   });
@@ -39,6 +47,10 @@ describe('toRowId', () => {
     expect(toRowId('42')).toBe(42);
   });
 
+  it('ROWID-009B: accepts a leading-zero digit string ("007") — SQLite\'s own affinity conversion resolves this to integer 7 too (verified against better-sqlite3), and the guard\'s regex (/^\\d+$/) must keep matching it: never tighten it to /^(0|[1-9]\\d*)$/', () => {
+    expect(toRowId('007')).toBe(7);
+  });
+
   it('ROWID-010: accepts an already-numeric safe integer', () => {
     expect(toRowId(42)).toBe(42);
   });