reminder_log_kind_check Regex Test Was a False Green

The short version

There was a static test that pulled the allowed values of a DB CHECK constraint out of a SQL file with a regex. The regex was written as “once the constraint name shows up anywhere, grab the first check (kind in (...)) found after it,” so it could just as easily pick up a line that only mentions the constraint name in a comment, or an identically shaped check (kind in (...)) on an unrelated table. The fix came in two stages: (a) anchor the match on the real add constraint <name> check (...) DDL clause instead of a bare mention of the name, and (b) since the same clause could still be quoted inside a SQL comment, strip comment lines with a regex before searching. Both landed from the same day’s code review, in two commits eleven minutes apart.

What the test was for

The subject is the reminder_log table in kimiteras-portal. It logs reminder and follow-up emails to prevent sending duplicates, storing strings like 'unsigned' and 'renewal' in a kind column. The TypeScript side defines the allowed values as a union type called ReminderKind; the DB side declares what may be inserted via a CHECK constraint named reminder_log_kind_check. Since the two live in separate files, keeping them in sync by hand was always going to drift eventually.

It already had. The comment at the top of the test file records what happened.

// Background (latent bug from 2026-07-02): customer-reminders.ts had been INSERTing
//   kind='overdue' since migration 0052, but the constraint never allowed 'overdue'
//   (0052/0064) -> every overdue INSERT hit a CHECK violation -> claimReminder=false
//   -> remind_overdue silently sent zero emails. 0065 allowed 'overdue'. This test now
//   catches drift between the code-side union and the DB constraint going forward.

customer-reminders.ts had been inserting kind='overdue' since migration 0052, but reminder_log_kind_check never allowed 'overdue' until 0065. Every insert failed its CHECK constraint, and because the caller was a best-effort design that swallowed the exception in a try/catch, overdue reminder emails went out zero times, silently. Migration 0065 added 'overdue' to the allowed values to fix it, and reminder-log-kind-constraint.test.ts was written right after, as a pre-build regression check that the code-side union and the DB-side constraint still agree, so the same drift could never happen unnoticed again.

The cause

The test’s allowedKindsFromMigrations() reads every .sql file under supabase/migrations/ in filename order (zero-padded numbers, so lexical order equals apply order), and pulls the latest allowed values out of each file with a regex. At the time it was introduced (2026-07-12), the implementation looked like this:

function allowedKindsFromMigrations(): string[] {
  const files = readdirSync(migrationsDir)
    .filter((f) => f.endsWith(".sql"))
    .sort(); // 0052 < 0064 < 0065 ... zero-padded, so lexical order = apply order
  let allowed: string[] | null = null;
  for (const f of files) {
    const sql = readFileSync(`${migrationsDir}/${f}`, "utf8");
    // Take the last `... reminder_log_kind_check ... check (kind in ('a','b',...))` definition found.
    const re = /reminder_log_kind_check[\s\S]*?check\s*\(\s*kind\s+in\s*\(([^)]*)\)/gi;
    let mm: RegExpExecArray | null;
    while ((mm = re.exec(sql)) !== null) {
      allowed = [...mm[1].matchAll(/'([^']+)'/g)].map((x) => x[1]);
    }
  }
  if (!allowed) throw new Error("no check definition for reminder_log_kind_check found in migrations");
  return allowed;
}

This regex starts from the literal string reminder_log_kind_check and, non-greedily ([\s\S]*?), grabs the first check (kind in (...)) it finds after that. The problem is that the anchor — the string reminder_log_kind_check — doesn’t have to come from a real DDL definition (add constraint reminder_log_kind_check ...); a bare mention in a comment matches just as well. The fix commit’s inline comments spell out the two ways this breaks:

1. If a migration merely mentions the name in a comment and then contains a different check (kind in (...)) (e.g. contracts.kind), that unrelated one can be misread as the allowed set
2. If a migration follows this repo’s habit of quoting a past definition in a comment for context, the commented-out definition itself gets picked up

Case (1) actually happened. 0070_sponsor_contracts.sql contains an unrelated same-shaped clause for a different table, before it re-defines reminder_log_kind_check further down:

alter table public.contracts
  add column if not exists kind text not null default 'standard'
    check (kind in ('standard', 'sponsor'));

-- Guard against sending the migration-notice email twice in one day
-- (same convention as 0052/0064/0065 for adding allowed values).
alter table public.reminder_log drop constraint if exists reminder_log_kind_check;
alter table public.reminder_log
  add constraint reminder_log_kind_check
  check (kind in ('unsigned','unsubmitted','renewal','renewal_60','renewal_7','creative_expiry','overdue','sponsor_transition'));

Within this one file, the anchor string reminder_log_kind_check first appears right after the comment (the reminder_log drop/add), and the contracts.kind clause sits earlier in the file, before that anchor. Since the non-greedy match never looks backward from its anchor, this particular file caused no harm on its own. But allowedKindsFromMigrations() overwrites allowed across files as it goes, keeping only whatever matched in the last file processed. If some migration numbered after 0070 had left a light comment mention of reminder_log_kind_check (case 2) followed later in the same file by a check (kind in (...)) unrelated to reminder_log, that unrelated clause would silently overwrite the real result, and the test would still pass. At the time, 0070 was the last migration and it happened to carry the correct definition, so nothing broke — but that was only because the file processed last happened to be right, and the regex’s own design was fragile to file-processing order regardless.

The fix (stage 1)

A commit at 2026-07-24 15:55 changed the anchor from “the constraint name is mentioned” to “the actual DDL clause is present”:

// Take the last **definition** matching `add constraint reminder_log_kind_check check (kind in ('a','b',...))`.
// Do not let this anchor on a bare mention of the constraint name: a migration that only
// mentions the name in a comment could contain an unrelated `check (kind in (...))`
// (e.g. contracts.kind), which would then be misread as the allowed set.
// Picking up too broad a set silently produces a **false green** (the test passes while
// validating the wrong constraint), letting through the exact 0065-style gap
// (overdue reminders failing to insert) this test exists to catch.
const re =
  /add\s+constraint\s+reminder_log_kind_check\s+check\s*\(\s*kind\s+in\s*\(([^)]*)\)/gi;

Instead of anchoring on the bare string reminder_log_kind_check, the match now requires the full DDL clause add constraint reminder_log_kind_check check (kind in (...)). A comment that only mentions the name, or an unrelated table’s check (kind in (...)), no longer matches unless it forms this exact sequence.

The fix (stage 2)

But stage 1 still had a hole. A JS regex has no concept of SQL’s -- comment syntax. Even if the literal string add constraint reminder_log_kind_check check (kind in (...)) sits after a --, inside a comment, the regex matches it exactly the same as real DDL. This repo has a habit — visible in 0065_reminder_log_overdue.sql — of quoting a past allowed-values definition in a comment while explaining the history. If a future migration followed the same habit and quoted a past add constraint ... statement verbatim in a comment, stage 1’s fix alone could reproduce the same false green.

Eleven minutes later, a 16:06 commit fixed it by stripping comment lines before searching:

for (const f of files) {
  // Strip SQL comment lines before searching. This repo has a habit of quoting past
  // definitions in comments; picking up a commented-out definition would overwrite
  // the real latest one (last match wins) with a **false green**: the test passes
  // while validating a broader allowed set than production actually enforces.
  const sql = readFileSync(`${migrationsDir}/${f}`, "utf8").replace(
    /^[ \t]*--.*$/gm,
    ""
  );
  const re =
    /add\s+constraint\s+reminder_log_kind_check\s+check\s*\(\s*kind\s+in\s*\(([^)]*)\)/gi;
  let mm: RegExpExecArray | null;
  while ((mm = re.exec(sql)) !== null) {
    allowed = [...mm[1].matchAll(/'([^']+)'/g)].map((x) => x[1]);
  }
}

/^[ \t]*--.*$/gm blanks out any line that starts with -- (allowing leading whitespace) before the now-anchored regex runs. This lets the test tell “a real DDL definition” apart from “a comment that mentions or quotes one” by syntactic position, not by string shape alone.

What’s still unguarded

Both fixes only changed the logic inside allowedKindsFromMigrations() itself; neither of the two pitfalls it fixed — matching an unrelated table’s same-shaped clause, or matching a commented-out definition — has a dedicated it() that reproduces and verifies it directly. The two existing test cases only check “is every member of the union allowed” and “is 'overdue' in the allowed set,” both of which only confirm the current migration files happen to produce the right answer. A structural gap like this one, caused by how the regex chooses its anchor, can stay green purely because the current set of files doesn’t happen to trigger it. The next time someone writes a migration with an unrelated check (kind in (...)) on another table, or quotes a past definition in a comment, whether this fix still holds is left to the next code review, not to the test suite.