[codex] Fix mysql insert boundary handling (#364)

* fix mysql insert boundary handling

* fix review comment follow-ups

* fix account insert boundary regressions

src/server/db/index.proxy-wrap.test.ts

@@ -58,6 +58,29 @@ describe('db proxy query wrapper', () => {
     expect(testUtils.shouldWrapObject(Promise.resolve())).toBe(false);
   });
 
+  it('normalizes postgres inserts with returning ids for numeric-id tables', async () => {
+    const query = vi.fn(async () => ({
+      rows: [{ id: 42 }],
+      rowCount: 1,
+    }));
+    const executor = { query };
+
+    const result = await testUtils.pgProxyQuery(
+      executor as any,
+      'insert into "sites" ("name") values ($1)',
+      ['demo'],
+      'execute',
+    );
+
+    expect(query).toHaveBeenCalledWith({
+      text: 'insert into "sites" ("name") values ($1) returning id',
+      values: ['demo'],
+    });
+    expect(result).toEqual({
+      rows: [{ changes: 1, lastInsertRowid: 42 }],
+    });
+  });
+
   it('builds mysql pool options with jsonStrings enabled', () => {
     expect(testUtils.buildMysqlPoolOptions('mysql://root:pass@db.example.com:3306/metapi', false)).toMatchObject({
       uri: 'mysql://root:pass@db.example.com:3306/metapi',

src/server/db/index.ts

@@ -1398,6 +1398,7 @@ export async function switchRuntimeDatabase(nextDialect: RuntimeDbDialect, nextD
 export const __dbProxyTestUtils = {
   wrapQueryLike,
   shouldWrapObject,
+  pgProxyQuery,
   resolveSqlitePath,
   resolveVitestSqlitePath,
   buildMysqlPoolOptions,

src/server/db/insertHelpers.test.ts

@@ -0,0 +1,72 @@
+import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest';
+import { mkdtempSync, rmSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+
+type DbModule = typeof import('./index.js');
+type InsertHelpersModule = typeof import('./insertHelpers.js');
+
+describe('insert helpers', () => {
+  let db: DbModule['db'];
+  let schema: DbModule['schema'];
+  let insertHelpers: InsertHelpersModule;
+  let dataDir = '';
+
+  beforeAll(async () => {
+    dataDir = mkdtempSync(join(tmpdir(), 'metapi-insert-helpers-'));
+    process.env.DATA_DIR = dataDir;
+    await import('./migrate.js');
+    const dbModule = await import('./index.js');
+    db = dbModule.db;
+    schema = dbModule.schema;
+    insertHelpers = await import('./insertHelpers.js');
+  });
+
+  beforeEach(async () => {
+    await db.delete(schema.sites).run();
+  });
+
+  afterAll(() => {
+    delete process.env.DATA_DIR;
+    if (dataDir) {
+      rmSync(dataDir, { recursive: true, force: true });
+    }
+  });
+
+  it('extracts positive inserted ids from run results', () => {
+    expect(insertHelpers.getInsertedRowId({ changes: 1, lastInsertRowid: 12 })).toBe(12);
+    expect(insertHelpers.getInsertedRowId({ changes: 1, lastInsertRowid: 0 })).toBeNull();
+    expect(insertHelpers.getInsertedRowId(null)).toBeNull();
+  });
+
+  it('throws when a required inserted id is missing', () => {
+    expect(() => insertHelpers.requireInsertedRowId({ changes: 1, lastInsertRowid: 0 }, 'missing insert id'))
+      .toThrow('missing insert id');
+  });
+
+  it('inserts and reloads rows through the shared helper', async () => {
+    const created = await insertHelpers.insertAndGetById({
+      table: schema.sites,
+      idColumn: schema.sites.id,
+      values: {
+        name: 'Shared helper site',
+        url: 'https://shared-helper.example.com',
+        platform: 'codex',
+        status: 'active',
+        useSystemProxy: false,
+        isPinned: false,
+        globalWeight: 1,
+        sortOrder: 0,
+      },
+      insertErrorMessage: 'insert site failed',
+      loadErrorMessage: 'load site failed',
+    });
+
+    expect(created).toMatchObject({
+      id: expect.any(Number),
+      name: 'Shared helper site',
+      url: 'https://shared-helper.example.com',
+      platform: 'codex',
+    });
+  });
+});

src/server/db/insertHelpers.ts

@@ -0,0 +1,38 @@
+import { eq } from 'drizzle-orm';
+import { db } from './index.js';
+
+type InsertRunResult = {
+  changes?: number;
+  lastInsertRowid?: number;
+} | null | undefined;
+
+export function getInsertedRowId(result: InsertRunResult): number | null {
+  const insertedId = Number(result?.lastInsertRowid || 0);
+  return insertedId > 0 ? insertedId : null;
+}
+
+export function requireInsertedRowId(result: InsertRunResult, errorMessage: string): number {
+  const insertedId = getInsertedRowId(result);
+  if (insertedId == null) {
+    throw new Error(errorMessage);
+  }
+  return insertedId;
+}
+
+export async function insertAndGetById<T>(input: {
+  txDb?: typeof db;
+  table: any;
+  idColumn: any;
+  values: Record<string, unknown>;
+  insertErrorMessage: string;
+  loadErrorMessage?: string;
+}): Promise<T> {
+  const txDb = input.txDb ?? db;
+  const inserted = await txDb.insert(input.table).values(input.values).run();
+  const insertedId = requireInsertedRowId(inserted, input.insertErrorMessage);
+  const created = await txDb.select().from(input.table).where(eq(input.idColumn, insertedId)).get();
+  if (!created) {
+    throw new Error(input.loadErrorMessage ?? input.insertErrorMessage);
+  }
+  return created as T;
+}

src/server/db/returning.architecture.test.ts

@@ -0,0 +1,48 @@
+import { readdirSync, readFileSync, statSync } from 'node:fs';
+import { join, relative, sep } from 'node:path';
+import { fileURLToPath } from 'node:url';
+import { describe, expect, it } from 'vitest';
+
+const SERVER_ROOT = fileURLToPath(new URL('../', import.meta.url));
+const FORBIDDEN_PATTERN = '.returning(';
+const INSERT_ID_PATTERN = 'lastInsertRowid';
+
+function collectProductionServerFiles(rootDir: string): string[] {
+  const entries = readdirSync(rootDir).sort();
+  const files: string[] = [];
+
+  for (const entry of entries) {
+    const absolutePath = join(rootDir, entry);
+    const stats = statSync(absolutePath);
+    if (stats.isDirectory()) {
+      files.push(...collectProductionServerFiles(absolutePath));
+      continue;
+    }
+    if (!entry.endsWith('.ts')) continue;
+    if (entry.endsWith('.test.ts')) continue;
+    files.push(absolutePath);
+  }
+
+  return files;
+}
+
+describe('server database dialect architecture boundaries', () => {
+  it('keeps production server code free of drizzle builder returning calls', () => {
+    const rootDir = SERVER_ROOT;
+    const offenders = collectProductionServerFiles(rootDir)
+      .filter((filePath) => readFileSync(filePath, 'utf8').includes(FORBIDDEN_PATTERN))
+      .map((filePath) => relative(rootDir, filePath));
+
+    expect(offenders).toEqual([]);
+  });
+
+  it('keeps lastInsertRowid handling inside the db layer', () => {
+    const rootDir = SERVER_ROOT;
+    const offenders = collectProductionServerFiles(rootDir)
+      .filter((filePath) => !relative(rootDir, filePath).split(sep).join('/').startsWith('db/'))
+      .filter((filePath) => readFileSync(filePath, 'utf8').includes(INSERT_ID_PATTERN))
+      .map((filePath) => relative(rootDir, filePath));
+
+    expect(offenders).toEqual([]);
+  });
+});