fix(18): WR-02 derive seed flag from DB write, not a stale pre-flight SELECT

The seed handler computed seeded from a pre-flight SELECT then returned
seeded:!alreadySet. Under a genuine concurrent race both requests can
SELECT the empty table, both enter the insert branch, and both return
seeded:true though only one row was actually written. Replace the
SELECT + conditional onDuplicateKeyUpdate with a single INSERT IGNORE
and derive seeded from affectedRows (1 = inserted, 0 = ignored/existing
row preserved, D-03). On MariaDB onDuplicateKeyUpdate(value=value)
reports affectedRows 1 for both insert and no-op, so it cannot
distinguish them; INSERT IGNORE can. timezone is bound via a
parameterized sql template and is already IANA-validated by zod. Adds a
test asserting seeded:false for a directly-pre-inserted row.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Lucas Berger
2026-06-15 07:41:52 -04:00
co-authored by Claude Opus 4.8
parent 692fe2ad9a
commit 93217b58fe
2 changed files with 49 additions and 23 deletions
+18 -23
View File
@@ -241,34 +241,29 @@ adminRouter.put('/config/timezone', zValidator('json', timezoneSchema), async (c
// explicit choice. // explicit choice.
// //
// Always returns 200 with { ok: true, seeded: <bool> }. // Always returns 200 with { ok: true, seeded: <bool> }.
// Does NOT use onDuplicateKeyUpdate — an explicit SELECT + conditional INSERT // Uses a single INSERT IGNORE: when the household_timezone row already exists the
// ensures the existing value is never overwritten (D-03). // 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) => { adminRouter.post('/config/timezone/seed', zValidator('json', timezoneSchema), async (c) => {
const { timezone } = c.req.valid('json'); const { timezone } = c.req.valid('json');
const [existing] = await db // WR-02: always run the INSERT and let the DB be the source of truth, instead of a
.select({ value: appConfig.value }) // pre-flight SELECT whose result could be stale under a concurrent race (two racers
.from(appConfig) // both observing an empty table and both returning seeded:true). INSERT IGNORE on
.where(eq(appConfig.key, 'household_timezone')) // MariaDB reports affectedRows === 1 for a real insert and 0 when the row already
.limit(1); // 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` }`) return c.json({ ok: true, seeded }, 200);
// 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);
}); });
+31
View File
@@ -813,4 +813,35 @@ describe('admin timezone config', () => {
.limit(1); .limit(1);
expect(row?.value).toBe('America/New_York'); 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');
});
}); });