diff --git a/apps/api/src/routes/admin.ts b/apps/api/src/routes/admin.ts index 5f50ca5..1f25f82 100644 --- a/apps/api/src/routes/admin.ts +++ b/apps/api/src/routes/admin.ts @@ -241,34 +241,29 @@ adminRouter.put('/config/timezone', zValidator('json', timezoneSchema), async (c // explicit choice. // // Always returns 200 with { ok: true, seeded: }. -// Does NOT use onDuplicateKeyUpdate — an explicit SELECT + conditional INSERT -// ensures the existing value is never overwritten (D-03). +// Uses a single INSERT IGNORE: when the household_timezone row already exists the +// insert is silently ignored (existing value untouched — D-03 no-overwrite) and +// cannot 500 on the PK constraint under a concurrent seed/PUT. `seeded` is derived +// from the result's affectedRows so it reflects what the DB actually did, accurately +// even under a genuine concurrent race (WR-02). // --------------------------------------------------------------------------- adminRouter.post('/config/timezone/seed', zValidator('json', timezoneSchema), async (c) => { const { timezone } = c.req.valid('json'); - const [existing] = await db - .select({ value: appConfig.value }) - .from(appConfig) - .where(eq(appConfig.key, 'household_timezone')) - .limit(1); + // WR-02: always run the INSERT and let the DB be the source of truth, instead of a + // pre-flight SELECT whose result could be stale under a concurrent race (two racers + // both observing an empty table and both returning seeded:true). INSERT IGNORE on + // MariaDB reports affectedRows === 1 for a real insert and 0 when the row already + // exists (ignored, value preserved — D-03), so deriving `seeded` from affectedRows + // is accurate: only the racer whose INSERT actually wrote the row gets seeded:true. + // `timezone` is interpolated via drizzle's parameterized sql template (bound param, + // not string concatenation) and is already validated as an IANA zone by timezoneSchema. + const result = (await db.execute( + sql`INSERT IGNORE INTO ${appConfig} (${sql.identifier('key')}, ${sql.identifier('value')}) VALUES ('household_timezone', ${timezone})`, + )) as unknown as [{ affectedRows: number }, unknown]; - const alreadySet = existing?.value != null; + const seeded = result[0].affectedRows === 1; - // WR-02: Use onDuplicateKeyUpdate with a no-op (`set: { value: sql`value` }`) - // so a concurrent seed or a seed racing a PUT cannot 500 on the PK constraint. - // The no-op preserves the existing value (D-03 no-overwrite). We always INSERT - // here and let the DB determine whether a row was inserted or not; `seeded` still - // reflects the pre-flight SELECT so the caller gets the correct flag even in the - // concurrent race (the winner observes alreadySet=false → seeded:true; the loser - // observes alreadySet=true → seeded:false and the INSERT is a no-op). - if (!alreadySet) { - await db - .insert(appConfig) - .values({ key: 'household_timezone', value: timezone }) - .onDuplicateKeyUpdate({ set: { value: sql`value` } }); - } - - return c.json({ ok: true, seeded: !alreadySet }, 200); + return c.json({ ok: true, seeded }, 200); }); diff --git a/apps/api/tests/routes/admin.test.ts b/apps/api/tests/routes/admin.test.ts index ef58fc6..d972e05 100644 --- a/apps/api/tests/routes/admin.test.ts +++ b/apps/api/tests/routes/admin.test.ts @@ -813,4 +813,35 @@ describe('admin timezone config', () => { .limit(1); expect(row?.value).toBe('America/New_York'); }); + + // ------------------------------------------------------------------------- + // POST seed — `seeded` flag is derived from what the DB actually did (WR-02) + // ------------------------------------------------------------------------- + + it('POST seed reports seeded:false when a row was pre-inserted directly, not via the endpoint (WR-02 accurate flag)', async () => { + const adminId = await seedUser('tz-admin-seed-derived', true); + currentDevUserId = adminId; + const app = await getApp(); + + // Insert the row directly (bypassing the seed endpoint) so no request-scoped + // pre-flight SELECT could have observed "unset". A correct implementation must + // derive seeded from the INSERT result (affectedRows), so this returns false. + await db.insert(appConfig).values({ key: 'household_timezone', value: 'America/Denver' }); + + const res = await app.fetch( + jsonRequest('POST', '/api/admin/config/timezone/seed', { timezone: 'Asia/Tokyo' }), + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { ok: boolean; seeded: boolean }; + expect(body.ok).toBe(true); + expect(body.seeded).toBe(false); + + // D-03 preserved: the directly-inserted value is untouched. + const [row] = await db + .select({ value: appConfig.value }) + .from(appConfig) + .where(eq(appConfig.key, 'household_timezone')) + .limit(1); + expect(row?.value).toBe('America/Denver'); + }); });