From f70018267450094e397474b41cff43ad6f93b7ed Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Sun, 7 Jun 2026 18:14:20 -0400 Subject: [PATCH] fix(auth): assign first UNUSED palette color (AUTH-03 distinct colors) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gate 2 A3 fail: a second member (amelia) got the same color as the first (luc), both #E8734A. Color was assigned by COUNT(*) % palette; a deleted spike user shifted the count so two live members landed on the same slot. Replace with 'first palette color not already in use by another user' (fall back to count round-robin only once the palette is exhausted) — guarantees distinct, stable colors for up to palette length members. +1 regression test (deletion frees a slot → next member fills it, no collision). --- apps/api/src/auth/user.ts | 21 +++++++++------ apps/api/tests/auth/user.test.ts | 44 ++++++++++++++++++++++++++++---- 2 files changed, 52 insertions(+), 13 deletions(-) diff --git a/apps/api/src/auth/user.ts b/apps/api/src/auth/user.ts index b2e0c79..3a0339f 100644 --- a/apps/api/src/auth/user.ts +++ b/apps/api/src/auth/user.ts @@ -8,7 +8,7 @@ * Source: RESEARCH.md § "User upsert with color assignment" */ -import { and, eq, sql } from 'drizzle-orm' +import { and, eq } from 'drizzle-orm' import { db } from '../db/client.js' import { users } from '../db/schema.js' @@ -101,13 +101,18 @@ export async function upsertUser( return existing[0] } - // 2. Count existing users to determine round-robin color slot - const countResult = await db - .select({ count: sql`COUNT(*)` }) - .from(users) - - const count = Number(countResult[0]?.count ?? 0) - const color = COLOR_PALETTE[count % COLOR_PALETTE.length] + // 2. Assign the first palette color NOT already in use by another member. + // A plain COUNT(*) % palette collides under deletions: a deleted user + // shifts the count so the next insert reuses an in-use slot (observed in + // Gate 2 — two members both got #E8734A). Selecting the first unused color + // guarantees distinct, stable colors for up to COLOR_PALETTE.length members + // (AUTH-03). Falls back to round-robin by count only once the palette is + // exhausted (more members than colors). + const usedRows = await db.select({ color: users.color }).from(users) + const usedColors = new Set(usedRows.map((r) => r.color)) + const color = + COLOR_PALETTE.find((c) => !usedColors.has(c)) ?? + COLOR_PALETTE[usedColors.size % COLOR_PALETTE.length] // 3. Insert new user row // mysql2 has no RETURNING clause — use $returningId() then re-select diff --git a/apps/api/tests/auth/user.test.ts b/apps/api/tests/auth/user.test.ts index 0d9aae3..560f3ed 100644 --- a/apps/api/tests/auth/user.test.ts +++ b/apps/api/tests/auth/user.test.ts @@ -68,7 +68,7 @@ describe('upsertUser', () => { const sub = 'user-sub-001' // First select: no existing user - // Second select (count): count = 0 + // Second select (used colors): no existing users → no colors in use → palette[0] // Third select (re-fetch after insert): return the inserted row let selectCallCount = 0 mockDb.select.mockImplementation(() => { @@ -78,9 +78,9 @@ describe('upsertUser', () => { return makeSelectChain([]) } if (selectCallCount === 2) { - // COUNT(*) query — 0 users + // Used-colors query — no existing users return { - from: vi.fn().mockResolvedValue([{ count: 0 }]), + from: vi.fn().mockResolvedValue([]), } } // Re-fetch after insert @@ -109,9 +109,10 @@ describe('upsertUser', () => { return makeSelectChain([]) // not found } if (selectCallCount === 2) { - // COUNT(*) — 1 existing user + // Used-colors query — one existing user already holds palette[0], + // so the next member must get the first unused color: palette[1]. return { - from: vi.fn().mockResolvedValue([{ count: 1 }]), + from: vi.fn().mockResolvedValue([{ color: COLOR_PALETTE[0] }]), } } return makeSelectChain([ @@ -126,6 +127,39 @@ describe('upsertUser', () => { expect(user!.color).toBe(COLOR_PALETTE[1]) }) + // Regression (Gate 2): a new member must get a color NOT already in use, even + // after a deletion. The old COUNT(*) % palette logic reused an in-use slot + // when the user count had shifted (two members both got #E8734A). With colors + // [0] and [2] taken (slot [1] freed by a delete), the next member fills [1]. + it('assigns the first UNUSED palette color (no collision after deletions)', async () => { + const iss = 'https://auth.example.com' + const sub = 'user-sub-005' + + let selectCallCount = 0 + mockDb.select.mockImplementation(() => { + selectCallCount++ + if (selectCallCount === 1) return makeSelectChain([]) // not found + if (selectCallCount === 2) { + // palette[0] and palette[2] in use; palette[1] is free + return { + from: vi + .fn() + .mockResolvedValue([{ color: COLOR_PALETTE[0] }, { color: COLOR_PALETTE[2] }]), + } + } + return makeSelectChain([ + { id: 5, oidcIss: iss, oidcSub: sub, displayName: null, color: COLOR_PALETTE[1], createdAt: new Date() }, + ]) + }) + mockDb.insert.mockReturnValue(makeInsertChain([{ id: 5 }])) + + await upsertUser(iss, sub) + + // The inserted row's color must be the first unused palette entry (palette[1]). + const insertValues = mockDb.insert.mock.results[0]?.value?.values.mock.calls[0]?.[0] + expect(insertValues.color).toBe(COLOR_PALETTE[1]) + }) + it('returns the same user row on re-upsert (idempotent — no duplicate insert)', async () => { const iss = 'https://auth.example.com' const sub = 'user-sub-001'