test(db/postgres): cover withTransaction, lowercased backfill and updateRepo case handling

Author: dcoric

test/db/postgres/helper.test.ts

@@ -63,8 +63,14 @@ vi.mock('../../../src/config', () => ({
 }));
 
 describe('PostgreSQL - helper', async () => {
-  const { connect, query, resetConnection, getSessionStore, ensureSessionStoreReady } =
-    await import('../../../src/db/postgres/helper');
+  const {
+    connect,
+    query,
+    resetConnection,
+    getSessionStore,
+    ensureSessionStoreReady,
+    withTransaction,
+  } = await import('../../../src/db/postgres/helper');
 
   beforeEach(async () => {
     vi.clearAllMocks();
@@ -288,6 +294,47 @@ describe('PostgreSQL - helper', async () => {
     });
   });
 
+  describe('withTransaction', () => {
+    it('wraps the callback in BEGIN/COMMIT and releases the client', async () => {
+      getDatabaseMock.mockReturnValue({
+        type: 'postgres',
+        enabled: true,
+        connectionString: 'postgresql://localhost/x',
+      });
+
+      const result = await withTransaction(async (client) => {
+        await client.query('SELECT 1');
+        return 'ok';
+      });
+
+      expect(result).toBe('ok');
+      const statements = mockClientQuery.mock.calls.map(([sql]) => sql);
+      expect(statements[0]).toBe('BEGIN');
+      expect(statements).toContain('SELECT 1');
+      expect(statements[statements.length - 1]).toBe('COMMIT');
+      expect(mockClientRelease).toHaveBeenCalledTimes(1);
+    });
+
+    it('rolls back and rethrows when the callback fails', async () => {
+      getDatabaseMock.mockReturnValue({
+        type: 'postgres',
+        enabled: true,
+        connectionString: 'postgresql://localhost/x',
+      });
+
+      await expect(
+        withTransaction(async () => {
+          throw new Error('boom');
+        }),
+      ).rejects.toThrow('boom');
+
+      const statements = mockClientQuery.mock.calls.map(([sql]) => sql);
+      expect(statements[0]).toBe('BEGIN');
+      expect(statements[statements.length - 1]).toBe('ROLLBACK');
+      expect(mockClientRelease).toHaveBeenCalledTimes(1);
+    });
+  });
+
   describe('pool error handling', () => {
     it('registers an idle-client error listener that logs without crashing', async () => {
       getDatabaseMock.mockReturnValue({

test/db/postgres/repo.test.ts

@@ -383,4 +383,15 @@ describe('PostgreSQL - Repo', async () => {
       expect(repos[0].lastModified).toBe('2026-01-02T00:00:00.000Z');
     });
   });
+
+  it('lowercases usernames when replacing permissions through updateRepo', async () => {
+    mockQuery.mockResolvedValue({ rowCount: 1, rows: [] });
+
+    await updateRepo({ _id: 'r1', users: { canPush: ['Alice'], canAuthorise: ['BOB'] } });
+
+    const inserted = mockQuery.mock.calls
+      .filter(([sql]) => /INSERT INTO repo_users/.test(String(sql)))
+      .map(([, params]) => params?.[1]);
+    expect(inserted).toEqual(['alice', 'bob']);
+  });
 });

test/db/postgres/schemaMigrations.integration.test.ts

@@ -131,6 +131,9 @@ describe.runIf(shouldRunPostgresTests)('PostgreSQL Schema Migration Integration
       const single = await seed('single', { canPush: ['alice'], canAuthorise: ['bob'] });
       const multi = await seed('multi', { canPush: ['amy', 'cara'], canAuthorise: ['dan'] });
       const both = await seed('both', { canPush: ['eve'], canAuthorise: ['eve'] });
+      // Legacy entries can be mixed case; the runtime writers lowercase, so the
+      // backfill must too or these users become unretrievable.
+      const mixed = await seed('mixed', { canPush: ['Alice'], canAuthorise: ['ALICE', 'alice'] });
       const empty = await seed('empty', { canPush: [], canAuthorise: [] });
 
       // Apply the remaining migrations: v3 creates repo_users + backfills, v4
@@ -171,6 +174,12 @@ describe.runIf(shouldRunPostgresTests)('PostgreSQL Schema Migration Integration
         { username: 'eve', role: 'canAuthorise' },
         { username: 'eve', role: 'canPush' },
       ]);
+
+      // Mixed-case entries are lowercased and case-only duplicates collapse.
+      expect(await permsOf(mixed)).toEqual([
+        { username: 'alice', role: 'canAuthorise' },
+        { username: 'alice', role: 'canPush' },
+      ]);
       // A repo with no permissions backfills nothing.
       expect(await permsOf(empty)).toEqual([]);
     } finally {

test/db/postgres/schemaMigrations.test.ts

@@ -117,4 +117,9 @@ describe('PostgreSQL - migrations', () => {
     await expect(runMigrations(pool as never)).rejects.toThrow('migration boom');
     expect(release).toHaveBeenCalledTimes(1);
   });
+
+  it('lowercases usernames in the repo_users backfill to match the runtime writers', () => {
+    const v4 = MIGRATIONS.find((m) => m.version === 4);
+    expect(v4?.sql).toContain('lower(elem.username)');
+  });
 });