read-merge-write Drops a Key Without FOR UPDATE

The short version

A read-merge-write — “read the existing row, splice in one key on the JS side, write the whole thing back with an UPSERT” — loses updates under concurrent saves unless the read takes a row lock. This bites hardest when several keys share one row and each can be saved independently, at different times. The fix: turn the read of the existing value into a SELECT ... FOR UPDATE inside the same transaction, and hold the row lock until the write completes.

What it looks like

The target is saveAssignmentDeadlineFormatAction, a Server Action that saves school-scoped display settings (the school_configs table, scope='school', kind='display_settings'). That single row’s value column co-locates more than one key — not just the assignment deadline display format (assignmentDeadlineFormat), but also the school’s default design (signageDesign) and the editor’s day-cutover time (editorDayCutover). Because an UPSERT replaces the entire value column, the save Action has to read the existing value, splice in only the key it wants to change, and write the merged object back — otherwise it would blow away every other key.

The code as first introduced (2026-07-11) used a plain, unlocked SELECT for that read:

const prev = await getSchoolConfigValue(tx, "display_settings");
const operation: "insert" | "update" = prev === null ? "insert" : "update";
const before = parseAssignmentDeadlineFormat(prev);
const base =
  prev && typeof prev === "object" && !Array.isArray(prev)
    ? (prev as Record<string, unknown>)
    : {};

const id = await upsertSchoolConfig(tx, {
  schoolId: actor.schoolId,
  kind: "display_settings",
  value: { ...base, assignmentDeadlineFormat: rawFormat },
  actorUserId,
});

Suppose the Action saving assignmentDeadlineFormat and a separate Action saving signageDesign or editorDayCutover both get called, near-simultaneously, against the same school’s same row. Each is an independent Server Action, so each runs its own “read → merge → write” inside its own transaction. If one transaction reads the existing value before the other has committed, its merge base never picks up the key change the other is about to commit. Whichever UPSERT commits later then overwrites the row wholesale with its own value — silently erasing the key change that had already landed.

Why

This is a textbook lost update: it trips no PostgreSQL constraint and surfaces no application error. Each save completes correctly on its own, and both callers get a success response. What’s broken isn’t either individual UPSERT — it’s the unstated assumption that nothing else can interleave between the read and the write. Under the default READ COMMITTED isolation level, an unlocked SELECT never establishes that guarantee.

The fix

The 2026-07-12 fix (#1264 / PR #1267) turned the read of the existing value into a SELECT ... FOR UPDATE inside the same transaction:

async function lockDisplaySettingsValue(tx: TenantTx): Promise<unknown | null> {
  const [row] = await tx
    .select({ value: schoolConfigs.value })
    .from(schoolConfigs)
    .where(and(eq(schoolConfigs.scope, "school"), eq(schoolConfigs.kind, "display_settings")))
    .limit(1)
    .for("update");
  return row ? row.value : null;
}

Callers just swap getSchoolConfigValue for lockDisplaySettingsValue(tx); the read-merge-write logic itself is unchanged:

const prev = await lockDisplaySettingsValue(tx);
const operation: "insert" | "update" = prev === null ? "insert" : "update";
const before = parseAssignmentDeadlineFormat(prev);
const base =
  prev && typeof prev === "object" && !Array.isArray(prev)
    ? (prev as Record<string, unknown>)
    : {};

const id = await upsertSchoolConfig(tx, {
  schoolId: actor.schoolId,
  kind: "display_settings",
  value: { ...base, assignmentDeadlineFormat: rawFormat },
  actorUserId,
});

SELECT ... FOR UPDATE locks the target row and holds that lock until the transaction ends. Any later transaction that tries to read the same row blocks on the SELECT itself until the lock releases, and then reads the just-committed latest value. That effectively serializes the two concurrent read-merge-writes, so one key change can no longer be swallowed by the other. When the row doesn’t exist yet (a first save), there’s nothing to lock and null comes back — a concurrent INSERT in that case is instead guarded by the unique constraint (ux_school_configs_target) together with onConflictDoUpdate.

What’s still open

The same commit adds a test, but it mocks the select().from().where().limit().for("update") chain with a fake tx and asserts only that the save Action called the mode string for("update"). It is a unit test, not an integration test that actually runs two transactions concurrently to reproduce the lost update and confirm it no longer reproduces after the fix. A query-builder mistake that only looks like it’s locking would likely slip past a test shaped this way.

The underlying design — several keys sharing one row — is unchanged. The fix holds only as long as every future Action writing to this row also goes through lockDisplaySettingsValue; a fourth writer added later that reads the row directly reopens the same race.