fix(auth): assign first UNUSED palette color (AUTH-03 distinct colors)

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).
This commit is contained in:
Lucas Berger
2026-06-07 18:14:20 -04:00
parent cdb097c5b3
commit f700182674
2 changed files with 52 additions and 13 deletions
+13 -8
View File
@@ -8,7 +8,7 @@
* Source: RESEARCH.md § "User upsert with color assignment" * 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 { db } from '../db/client.js'
import { users } from '../db/schema.js' import { users } from '../db/schema.js'
@@ -101,13 +101,18 @@ export async function upsertUser(
return existing[0] return existing[0]
} }
// 2. Count existing users to determine round-robin color slot // 2. Assign the first palette color NOT already in use by another member.
const countResult = await db // A plain COUNT(*) % palette collides under deletions: a deleted user
.select({ count: sql<number>`COUNT(*)` }) // shifts the count so the next insert reuses an in-use slot (observed in
.from(users) // Gate 2 — two members both got #E8734A). Selecting the first unused color
// guarantees distinct, stable colors for up to COLOR_PALETTE.length members
const count = Number(countResult[0]?.count ?? 0) // (AUTH-03). Falls back to round-robin by count only once the palette is
const color = COLOR_PALETTE[count % COLOR_PALETTE.length] // 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 // 3. Insert new user row
// mysql2 has no RETURNING clause — use $returningId() then re-select // mysql2 has no RETURNING clause — use $returningId() then re-select
+39 -5
View File
@@ -68,7 +68,7 @@ describe('upsertUser', () => {
const sub = 'user-sub-001' const sub = 'user-sub-001'
// First select: no existing user // 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 // Third select (re-fetch after insert): return the inserted row
let selectCallCount = 0 let selectCallCount = 0
mockDb.select.mockImplementation(() => { mockDb.select.mockImplementation(() => {
@@ -78,9 +78,9 @@ describe('upsertUser', () => {
return makeSelectChain([]) return makeSelectChain([])
} }
if (selectCallCount === 2) { if (selectCallCount === 2) {
// COUNT(*) query — 0 users // Used-colors query — no existing users
return { return {
from: vi.fn().mockResolvedValue([{ count: 0 }]), from: vi.fn().mockResolvedValue([]),
} }
} }
// Re-fetch after insert // Re-fetch after insert
@@ -109,9 +109,10 @@ describe('upsertUser', () => {
return makeSelectChain([]) // not found return makeSelectChain([]) // not found
} }
if (selectCallCount === 2) { 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 { return {
from: vi.fn().mockResolvedValue([{ count: 1 }]), from: vi.fn().mockResolvedValue([{ color: COLOR_PALETTE[0] }]),
} }
} }
return makeSelectChain([ return makeSelectChain([
@@ -126,6 +127,39 @@ describe('upsertUser', () => {
expect(user!.color).toBe(COLOR_PALETTE[1]) 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 () => { it('returns the same user row on re-upsert (idempotent — no duplicate insert)', async () => {
const iss = 'https://auth.example.com' const iss = 'https://auth.example.com'
const sub = 'user-sub-001' const sub = 'user-sub-001'