DEV Community: phi_blankslate

My Stripe idempotency key was correct. It just stopped working after 24 hours.

phi_blankslate — Wed, 07 Oct 2026 04:24:05 +0000

I added a reward feature that pays out through Stripe's customer balance, and I did what every payments guide tells you to do: I passed an idempotency key on the write that moves money. A security review pass then found a double-payment path anyway, and the key was not wrong. The key was correct, unique, and deterministic. It simply had an expiry date that my retry design ignored.

This post is about the gap between "I used an idempotency key" and "this operation is idempotent." They are not the same claim, and the distance between them is a time window.

The stack is Next.js, Drizzle, Neon and the Stripe Node SDK, but the lesson has nothing to do with any of those.

The feature

When a referred user's paid subscription goes through, the referrer gets a credit on their Stripe customer balance. In Stripe terms that is a negative customer balance transaction, which Stripe automatically applies to the customer's next invoice.

The reward logic lives in one function, rewardReferralIfEligible, and it can be called from three places: a webhook for customer.subscription.updated, a webhook for invoice.paid, and a manual sweep script I run to catch anything the webhooks missed. Because the same function can be reached from several triggers, it has to be safe to call repeatedly. That requirement is where the story starts.

There is a referrals table with a status column: pending or rewarded. The function's job is to move a row from pending to rewarded exactly once and, as a side effect of that single transition, create exactly one balance transaction on the referrer's Stripe customer.

Layer one: make the database the gate

The first thing I wrote was a compare-and-swap. Before touching Stripe, claim the row:

const updated = await db
  .update(referrals)
  .set({ status: "rewarded", rewardedAt: new Date() })
  .where(and(eq(referrals.id, referral.id), eq(referrals.status, "pending")))
  .returning();
if (updated.length === 0) return;

The WHERE status = 'pending' clause is the important part. If two webhook deliveries race, both run this UPDATE, but only one of them gets a row back from .returning(). The loser sees an empty array and exits. Postgres serializes the two updates on the same row, so exactly one caller proceeds. This also means the "already processed" check and the "claim" are one atomic statement instead of a read followed by a write, which is the classic time-of-check-to-time-of-use hole.

So far so good. The winner now owns the row and goes on to talk to Stripe.

Layer two: the idempotency key

The winner then creates the balance transaction. This is the call that actually moves money, so this is where I put the key:

const tx = await stripe.customers.createBalanceTransaction(
  referrerCustomerId,
  {
    amount: -REFERRAL_REWARD_CENTS,
    currency: "usd",
    description: `Referral reward — referred user converted to Pro Annual ${marker}`,
  },
  { idempotencyKey: `referral-reward-${referral.id}` }
);

The key is derived from referral.id, which is a UUID assigned once when the referral row is created and never changes. That is the right shape for a key: the same logical operation always produces the same key, and different operations never collide. If the HTTP response is lost and the call is retried, Stripe recognizes the key and returns the original result instead of creating a second transaction.

I stopped here at first and considered the problem solved. Database CAS stops concurrent callers. The idempotency key stops duplicate writes to Stripe. Two layers.

The rollback that made it dangerous

There is a third piece of code, and it is the one I wrote for a good reason. The CAS flips the row to rewarded before the Stripe call. That ordering is deliberate: it is what makes concurrent callers safe. But it means that if the Stripe call then fails, the database says "rewarded" while no credit exists. That is a silent loss for the referrer, and the row is now permanently skipped because it is no longer pending.

So the error path reverts the row (trimmed to the relevant lines):

} catch (err) {
  try {
    await db
      .update(referrals)
      .set({ status: "pending", rewardedAt: null })
      .where(eq(referrals.id, referral.id));
  } catch (rollbackErr) {
    console.error(
      `[referral] CRITICAL: rollback to pending also failed for referral ${referral.id}. ` +
        `Row is stuck as "rewarded" without an actual credit. Manual reconciliation required.`,
      { originalError: err, rollbackError: rollbackErr }
    );
    throw err;
  }
  throw err;
}

Rolling back to pending means the next trigger, whether it is a later webhook or my sweep script, will pick the row up again and retry. That is what I want for a clean failure like a network error before Stripe received anything.

Now put the two mechanisms next to each other and ask what happens in the ambiguous case.

An ambiguous failure is when the request reaches Stripe, Stripe creates the balance transaction, and the response never makes it back to me. A timeout, a dropped connection, a function that gets killed mid-flight. From my code's point of view the call threw. From Stripe's point of view the credit exists.

My catch block cannot tell these two cases apart. It sees an exception and rolls the row back to pending. Now the database says "not rewarded" and Stripe says "rewarded", and the row is waiting for a retry.

If the retry happens within 24 hours, the idempotency key saves me: Stripe sees the same key, returns the original transaction, and nothing is duplicated.

If the retry happens later, it does not.

The 24-hour window

Stripe's documentation says that keys can be removed from the system automatically once they are at least 24 hours old, and that Stripe generates a new request if a key is reused after the original was pruned. That is the whole mechanism. After the window, the same key means "new request," and a new request creates a new balance transaction.

I had read that sentence before. I had even written a comment in the code saying the key protects retries "within 24h." What I had not done is ask who controls how long a row can stay in pending after a rollback.

The answer was: not me, at least not tightly. A row reverted to pending waits for the next trigger. The webhook triggers are driven by Stripe's event delivery, which I do not control. The sweep script runs when I run it. If nothing touches that row for a day and a bit, and then the next trigger arrives, the function runs the whole sequence again with an expired key. The sequence passes every check I had written: the row is pending, the subscription is eligible, the CAS succeeds, and Stripe happily creates a second credit.

The reviewer's finding, stated plainly: a rollback-and-retry design plus a 24-hour key means the retry safety net has a hole that opens exactly when the system has been quiet the longest. And "quiet for a day" is not an exotic condition. It is what a weekend looks like.

This is the part worth sitting with. Every individual piece of the code was defensible. The CAS was correct. The key was correct. The rollback was correct. The bug lived in the interaction between the rollback's retry delay, which is unbounded, and the key's lifetime, which is bounded.

The fix: check Stripe's state, not your own memory

An idempotency key is a cache of "I already did this" held by someone else, with a TTL. When the question is "did this money already move," the authoritative answer is the ledger, not the cache. So before creating the balance transaction, I now look at what is actually on the referrer's customer balance:

const marker = `(referral ${referral.id})`;

const allTxs = await stripe.customers
  .listBalanceTransactions(referrerCustomerId, { limit: 100 })
  .autoPagingToArray({ limit: 1000 });
if (allTxs.length >= 1000) {
  throw new Error(
    `referrer ${referrerCustomerId} has 1000+ balance transactions; manual reconciliation required`
  );
}
const existing = allTxs.find((tx) => (tx.description ?? "").includes(marker));
if (existing) {
  console.warn(
    `[referral] credit already present on Stripe for referral ${referral.id}; marked rewarded without a new credit`
  );
  await recordBalanceTransactionId(referral.id, existing.id);
  return;
}

The marker is a string embedded in the description of every credit I create, containing the referral's own ID. If a credit with that marker already exists on the customer, the function does not create another one. It records the existing transaction's ID against the row and finishes. The state of the world is read from Stripe at the moment of the decision, so it does not matter how long the row sat in pending.

I kept the idempotency key. It is still the cheapest protection against the common case, a retry seconds after a flaky response, and it costs nothing. But it is now the first line of defense, not the only one. The ledger lookup is what is left when the key has expired.

Two things I want to be honest about, because this fix is not free of tradeoffs:

Matching on a description string is a convention, not a constraint. It works because I control every writer of these credits. If someone adds a credit by hand in the dashboard with a different description, it is invisible to the check, and if someone edits a description, the check can miss it. I treat it as a second line of defense, not a proof.
The scan is bounded. Listing every balance transaction for a customer is fine for a customer with a handful of them and not fine for one with thousands. I cap the scan at 1,000 entries, and if the cap is hit I throw instead of guessing, which sends the row to manual reconciliation. Failing closed seemed better than silently assuming "not found" on a list I did not finish reading.

A stronger anchor: record the transaction ID

The description marker is a search. A search can be wrong in ways a direct lookup cannot, so I added a column to the referrals table that stores the balance transaction ID returned by Stripe:

async function recordBalanceTransactionId(referralId: string, txId: string): Promise<void> {
  try {
    await db
      .update(referrals)
      .set({ stripeBalanceTransactionId: txId })
      .where(eq(referrals.id, referralId));
  } catch (err) {
    console.error(
      `[referral] CRITICAL: credit ${txId} granted but failed to record on referral ${referralId}. ` +
        `Row will show as rewardedWithoutCredit in KPI; reconcile manually.`,
      err
    );
  }
}

and the top of the function checks it before doing anything else:

if (referral.stripeBalanceTransactionId) {
  await db
    .update(referrals)
    .set({ status: "rewarded", rewardedAt: referral.rewardedAt ?? new Date() })
    .where(and(eq(referrals.id, referral.id), eq(referrals.status, "pending")));
  return;
}

If the transaction ID is already recorded, the credit exists by definition, and the function just repairs the status without calling Stripe at all.

Notice the comment I left in recordBalanceTransactionId: it deliberately does not rethrow. The credit has already been granted at that point. If recording the ID failed and I threw, the outer catch would roll the row back to pending, and I would have built a brand-new route to the exact double-credit I had just closed. The error handling for "the money moved but my bookkeeping failed" has to be different from the error handling for "the money did not move." Mixing them is how a safety mechanism turns into a hazard.

The related trap I found one pass later

While the same function was under review, a second finding turned up that has the same flavor: an assumption about what a Stripe field means.

The reward was originally gated on the subscription's status being active. That sounds like "the customer paid." It is not. When a trial ends, Stripe moves the subscription to active before the payment for the first invoice has been collected. For a short window the subscription is active and nothing has been paid, which means a status check alone can hand out a reward for money that never arrived.

The gate now looks at the subscription's latest invoice:

const sub = await stripe.subscriptions.retrieve(stripeSubscriptionId, {
  expand: ["latest_invoice"],
});
if (sub.status !== "active") return;
// ...
if (!latestInvoice || latestInvoice.status !== "paid" || (latestInvoice.amount_paid ?? 0) <= 0) {
  return;
}

An invoice that is paid with amount_paid > 0 is evidence that money moved. A subscription status is a state machine label. I also added invoice.paid as a trigger for the function so that the payment itself, not a status change, wakes it up. Because the function re-derives eligibility from Stripe on every call and returns quietly when the answer is "not yet," it is safe to trigger from any event, and a trigger that arrives too early costs nothing.

What I would tell myself before writing this

An idempotency key is a time-limited promise from someone else. Before relying on one, write down its TTL next to the longest delay your own system can introduce between an attempt and its retry. If the second number can exceed the first, the key is not your idempotency story. Mine could, because a rolled-back row waits for an external trigger I do not control.

Rollback plus retry is where idempotency actually gets tested. Straight-line code with a key looks airtight. The risk concentrates wherever you convert an ambiguous failure into a "try again later." An ambiguous failure means you do not know whether the side effect happened, and "later" is unbounded unless you bound it.

Ask the ledger, not the cache. When the side effect lives in someone else's system, the safest idempotency check reads that system's current state at the moment of the decision. It costs an extra API call. It also stays correct no matter how long the row sat, how many times it was retried, or whether a key was pruned in between.

Order your layers by what they protect against. In this function the database CAS stops concurrent callers, the idempotency key stops fast duplicate retries, the ledger lookup stops slow ones, and the stored transaction ID makes the repair path cheap. Each one covers a failure the others do not. Removing any single layer reintroduces a specific bug, so it helps to be able to say which one.

Treat "the money moved but my bookkeeping failed" as its own case. It must not share an error path with "the money did not move." Rolling back after a successful side effect is how you manufacture the duplicate you were trying to prevent.

Status fields are not receipts. If a decision depends on "was it paid," find the object that records payment, an invoice or a charge, and check that. A status can be reached by paths you did not anticipate.

The uncomfortable part

I want to be clear about how this was found: not by a test and not by production. A review pass that was specifically told to attack the money path read the rollback code and the key's lifetime side by side and asked how long a row could stay pending. I had the 24-hour fact in a code comment, one screen away from the bug, and still did not connect the two.

My tests would not have caught it either, because reproducing it needs an ambiguous failure followed by a gap of more than a day, and nobody writes a test with a 25-hour sleep. If a bug only exists across a time window longer than your test suite's patience, the fix has to come from reasoning about the window, not from waiting for a failure to show it to you.

The fix is a handful of extra lines. The expensive part was learning to read idempotencyKey as a claim with an expiry date instead of as a property of the operation.

drizzle-kit generated a DROP TABLE for a table that was still in production. drizzle-kit wasn't the problem.

phi_blankslate — Fri, 02 Oct 2026 12:27:09 +0000

I asked drizzle-kit for a migration that added two tables. The SQL it generated also contained a DROP TABLE ... CASCADE for a table that was still sitting in my production database, and an ADD COLUMN for a column that production already had.

Neither change had anything to do with the feature I was building. If I had applied the file as it came out, the first statement would have deleted a live table along with anything that depended on it. The second would have failed with "column already exists" and taken the rest of the migration with it, or left it half-applied, depending on how I ran it.

drizzle-kit did nothing wrong. It did exactly what it is designed to do. The problem was that I had three different ideas of "the current schema" in my project and assumed they were the same thing. This post explains how they drifted apart, why the diff came out the way it did, and the checks I now run before any generated migration goes near production.

The setup

The app is Next.js with Drizzle ORM on Postgres (Neon). The schema lives in one TypeScript file, lib/db/schema.ts. Migrations are generated with drizzle-kit generate into drizzle/migrations/, and drizzle-kit keeps a JSON snapshot of the schema next to each migration in drizzle/migrations/meta/.

I don't let tooling apply migrations to production. I don't use drizzle-kit push, and the header of my migration files literally says not to use drizzle-kit migrate either. Production changes are applied by hand, inside a transaction, after I've read the SQL. That rule is the only reason this story ends with a cleanup instead of a restore.

There is one more habit in the background that matters. For small additive changes I sometimes skipped the generator and wrote a one-off script that runs a single idempotent DDL statement. That habit is half of the bug.

How the drift happened

The baseline migration (0000) was generated at the end of July. At that point the schema had a waitlist_entries table for a pre-launch email list, and the reviews table did not have a suppressed_count column yet.

The next day I made two schema changes in one commit, and neither went through drizzle-kit generate.

Change 1: I deleted the waitlist feature. We had decided to launch directly instead of collecting a waitlist, so I removed the route, the form, the email-sending module, and the table definition from schema.ts:

 // ウェイトリスト登録（プレローンチ用リード収集）
-export const waitlistEntries = pgTable(
-  "waitlist_entries",
-  {
-    id: uuid("id").defaultRandom().primaryKey(),
-    email: text("email").notNull().unique(),
-    source: text("source"), // UTM等の登録元。任意
-    createdAt: timestamp("created_at").defaultNow().notNull(),
-  },
-  (table) => ({
-    emailIdx: uniqueIndex("waitlist_entries_email_idx").on(table.email),
-  })
-);

(The leftover Japanese comment above it says "waitlist signups (pre-launch lead collection)". It outlived the code it described, which is a small preview of the whole problem.)

I deleted the definition. I did not drop the table. In my head the feature was gone. In production the table was still there, empty and unreferenced.

Change 2: I added a column with a one-off script. In the same commit, reviews gained a column:

     commentsCount: integer("comments_count").default(0),
+    suppressedCount: integer("suppressed_count").default(0),
     tokensUsed: integer("tokens_used").default(0),

Instead of generating a migration, I added it to production with a small script that runs one statement:

await sql`
  ALTER TABLE reviews
    ADD COLUMN IF NOT EXISTS suppressed_count integer DEFAULT 0
`;

It's idempotent, it checks information_schema afterwards, and it worked. From production's point of view, everything was correct.

So now:

schema.ts had no waitlist table and did have suppressed_count.
Production still had the waitlist table and also had suppressed_count.
The latest drizzle snapshot (0000_snapshot.json) still had the waitlist table and did not have suppressed_count.

Nothing complained, because nothing compares those three. The app only reads schema.ts. Production only knows what has been executed against it. The snapshot only changes when you run generate.

More than a month later: `drizzle-kit generate`

In early September I built a referral feature that needed two new tables, referral_codes and referrals. I added them to schema.ts and ran npm run db:generate.

The generated file contained the two CREATE TABLE statements I expected, and two statements I didn't:

DROP TABLE "waitlist_entries" CASCADE;
ALTER TABLE "reviews" ADD COLUMN "suppressed_count" ...;

(I edited the generated file before committing it, so these are the two lines as I recorded them in my notes at the time, not a byte-for-byte copy of the original output.)

My first reaction was that drizzle-kit had somehow connected to the wrong database. It hadn't connected to any database.

Why the diff looks like that

This was the part I had wrong in my head for months: drizzle-kit generate does not diff your schema against your database. It diffs your schema against its own last snapshot.

The flow is:

Read schema.ts and build a schema object.
Read the most recent snapshot JSON in meta/.
Emit SQL for every difference between the two.
Write a new snapshot equal to the current schema.ts.

My drizzle.config.ts has a dbCredentials block with the production-shaped URL, which made it easy to believe that generate looks at the database. It doesn't. Those credentials are for commands that actually talk to a database (push, migrate, pull, studio).

Run the drift through that flow:

	Last snapshot (`0000`)	`schema.ts` now	Generated SQL
`waitlist_entries`	exists	gone	`DROP TABLE ... CASCADE`
`reviews.suppressed_count`	missing	exists	`ADD COLUMN`
`referral_codes`, `referrals`	missing	exist	`CREATE TABLE` x2

Every line is correct relative to the snapshot. The snapshot was the stale party, and it was stale because I had changed the schema twice without telling the generator.

I can confirm this from the repository now. grep over the snapshot files shows waitlist_entries present in 0000_snapshot.json and absent from 0001 onward, and suppressed_count absent from 0000 and present from 0001 onward. The snapshot quietly absorbed both changes at the moment I ran generate for an unrelated feature.

What each statement would have done

The DROP. In my case the damage would have been close to zero. Before deciding anything, I checked production: the table existed, and SELECT COUNT(*) on it returned an empty result set. But "it happened to be empty" is luck, not a safety property. Put any other "dead" table in that position (an audit log you stopped writing to, a table a cron job still reads, a table a teammate is about to wire up) and the same generate run produces the same line. CASCADE widens it further: it also drops whatever depends on the table, such as foreign keys from other tables and views.

The ADD COLUMN. This one would have failed loudly, since production already had the column, and Postgres rejects a plain ADD COLUMN for an existing column. Loud is better than silent, but it still matters where it fails. If the file is applied as a single transaction, the whole migration rolls back and you retry. If it's applied statement by statement (some SQL consoles autocommit each statement), you end up with whatever ran before the failure applied and whatever came after it not. That's a schema nobody designed.

My one-off script had used ADD COLUMN IF NOT EXISTS, which is idempotent. The generator emits a plain ADD COLUMN, because as far as the snapshot knows the column has never existed. The idempotency lived in the script, not in the migration history.

What I actually did

Checked production before touching anything. I queried information_schema.tables and information_schema.columns to confirm what really existed. The table was there, and the column was there.
Removed both unrelated statements from the generated file. The migration was narrowed to the referral tables only. A migration should do what its name says and nothing else.
Cleaned up the dead table as a separate, deliberate step. After confirming it was empty, and with explicit sign-off, I dropped waitlist_entries by hand. It was its own decision instead of a side effect of a referral feature.
Applied the migration by hand, in a transaction. The committed file is now wrapped in BEGIN; ... COMMIT; with a header comment explaining why. A later edit to the same file made the transaction even more important: it replaces an index with a unique version (DROP INDEX IF EXISTS followed by CREATE UNIQUE INDEX). In an autocommit console, if the create fails after the drop succeeds, you're left with no index at all, which breaks every ON CONFLICT that relied on it.

Because the snapshot had already been rewritten to match schema.ts, the next generate (for later changes) produced clean diffs. The drift was absorbed into history. Repository and production agreed again, but only because a human reconciled them.

Why the side door existed in the first place

It's worth being honest about why I was making schema changes outside the generator at all, because the reason wasn't laziness. It was friction, and friction is what pushes everyone toward side doors.

The header comment of my one-off column script explains it: I didn't use drizzle-kit push because push can stop and ask interactive questions (for example, whether it's allowed to truncate a table), and a command that waits for a keypress in a non-interactive context just hangs. So for a single additive column against production, a tiny script with ADD COLUMN IF NOT EXISTS, a dry-run mode by default, an explicit --apply flag, and an information_schema check afterwards felt safer than the official tool. For that one change, it was.

generate has the same trait. When I ran the referral migration, I first tried it from an automated, non-TTY session, and it stopped at an interactive prompt it couldn't get an answer to. It had to be run from a real terminal. That's a reasonable design, since some schema changes are genuinely ambiguous (is this a rename or a drop-and-create?) and a tool shouldn't guess. But it means the "proper" path costs more than the side door, and every time the proper path costs more, some changes will skip it.

The fix isn't to ban side doors. Sometimes a hand-written, idempotent statement really is the right way to change production. The fix is to make sure every side door ends with a step that updates the front door's records: after the script runs, run generate so the snapshot catches up, and commit the result as "already applied". The script changes the database. The generate run changes the history. You need both, or the history starts lying.

The check I already had, and why it wasn't enough

I already had a read-only preflight script that runs before any DDL. It opens a READ ONLY transaction, lists the tables in public, and compares them with what schema.ts defines. Here's the core of it (comments translated):

// Query inside a READ ONLY transaction.
// Even if code below ever tries to write, the database refuses it.
const { currentDatabase, tables } = await sql.begin(async (tx) => {
  await tx`SET TRANSACTION READ ONLY`;

  const [{ current_database }] = await tx<{ current_database: string }[]>`
    SELECT current_database()
  `;
  const rows = await tx<{ table_name: string }[]>`
    SELECT table_name
    FROM information_schema.tables
    WHERE table_schema = 'public' AND table_type = 'BASE TABLE'
    ORDER BY table_name
  `;

  return { currentDatabase: current_database, tables: rows.map((r) => r.table_name) };
});

and later:

const warnings: string[] = [];
const unexpected = tables.filter((t) => !ALL_EXPECTED.includes(t));
if (unexpected.length > 0) {
  warnings.push(`table exists that schema.ts does not define: ${unexpected.join(", ")}`);
}

That warning is exactly the signal for the waitlist half of this bug: a table in production that the code doesn't define. It would have fired. But a warning in a JSON report is something you have to go and read, and the preflight answers a different question ("am I connected to the right database?"). It checks tables, not columns, so it would never have caught the suppressed_count half. And it compares production with schema.ts, not with the snapshot. The snapshot is the third party that actually drives generate, and nothing was looking at it.

What I do now

None of this needs special tooling. It's mostly about treating a generated migration as a proposal and treating the snapshot as state that can go stale.

1. Read every generated statement and trace it back to your change. For each line, ask "which edit in this branch caused this?" If the answer is "none", stop. An unexplained DROP or ALTER means the snapshot and reality have diverged somewhere, and the fix is to find out where, not to delete the line and move on.

2. Treat DROP and CASCADE in a generated file as a hard stop. Any destructive statement gets checked against the live database (does it exist, does it have rows, what depends on it) before it's allowed to stay. If it stays, it should be its own reviewed change, not a side effect of something else.

3. Don't change the schema outside the generator, or if you do, close the loop. One-off scripts are fine and sometimes the right call for production. But if a column goes in by script, run generate immediately afterwards so the snapshot learns about it, and commit that migration marked as already applied. Otherwise the generator will "discover" the change later, inside somebody else's feature branch.

4. Removing a table from schema.ts is a migration, not a cleanup. Deleting the definition is the start of removing a table, not the end. Either drop it in the same change, through a reviewed migration, or leave the definition in place with a comment until you're ready to drop it.

5. Wrap hand-applied migrations in a transaction. Partial application is the failure mode that turns an ordinary error into an unplanned schema.

6. Check production against schema.ts periodically, not just before migrations. Tables and columns. An information_schema query is enough:

SELECT table_name, column_name, data_type, column_default
FROM information_schema.columns
WHERE table_schema = 'public'
ORDER BY table_name, ordinal_position;

Compare that list with your schema file. Anything that exists on only one side is drift that your next generate will try to "fix" for you.

The general lesson

Most migration tools that generate SQL from a declarative schema work this way. They have to compare against something, and if that something is a file in your repo rather than the live database, it can only be as accurate as the history you fed it. Every schema change made by hand, by script, from a SQL console, or by deleting code without dropping the table it described creates a gap between the snapshot and reality. The generator closes that gap the only way it can: by writing SQL that makes the database match the file, whether you meant to ask for that or not.

The dangerous part isn't the generator. It's the assumption that "the schema" is one thing. In my project it was three: what the code declares, what the tool's snapshot records, and what the database actually contains. They agree only as long as every change goes through the same door. The day one change uses a side door, the next person to use the front door gets a migration that "fixes" it.

So the rule I follow now is short: a generated migration is a diff against history, not against production. Read it like a diff from a teammate who has been away for a month, because in effect that's what it is.

A stray .env file broke exactly 13 pages of my Next.js build — and dev mode never saw it

phi_blankslate — Mon, 28 Sep 2026 08:37:19 +0000

npx next build was failing on precisely 13 pages, every time, with the same cryptic error:

TypeError: Invalid URL, input: ''

npm run dev worked fine. git stash back to a commit from weeks earlier — same 13 pages, same error. Whatever this was, it wasn't something I'd just written.

The git stash step mattered more than it might look. My first assumption, like most people's, was "I broke something recently." Stashing back past several days of commits and reproducing the exact same failure on the exact same 13 pages ruled that out immediately — whatever this was, it wasn't in the diff. It was in the environment, which meant it was invisible to git log, invisible to code review, and invisible to anyone just reading the codebase. That's the property that made this bug worth writing up: it wasn't a logic error anyone could have caught by reading code, because the code was never wrong.

This is the story of chasing that error down to a single leftover file from a debugging session a month prior, and why the fix required understanding something about how Next.js loads .env* files that I'd never had a reason to learn before.

The setup

The app is a Next.js 14 (App Router) SaaS with NextAuth for GitHub OAuth login. app/layout.tsx is the root layout every page goes through, and it calls getServerSession() so the UI can tell if you're logged in:

export default async function RootLayout({ children }: { children: React.ReactNode }) {
  let session = null;
  try {
    session = await getServerSession(authOptions);
  } catch (error) {
    if ((error as { digest?: string })?.digest === "DYNAMIC_SERVER_USAGE") {
      throw error;
    }
    console.error("[layout] getServerSession failed (likely missing env vars):", error);
  }

  return (
    <html lang="en">
      <body>
        <Providers session={session}>{children}</Providers>
      </body>
    </html>
  );
}

Note the try/catch there. I'd added it specifically so that missing env vars during a static build wouldn't crash the whole page — log it, fall back to session = null, move on. It felt like it should be bulletproof. It was not the code that was crashing.

Providers is a small client component:

"use client";
import { SessionProvider } from "next-auth/react";

export function Providers({ children, session }) {
  return <SessionProvider session={session}>{children}</SessionProvider>;
}

Thirteen lines, no logic. And yet this is where the trail eventually led.

Two wrong guesses, then I stopped guessing

My first instinct was: something's missing from .env.local. I checked NEXTAUTH_URL — it was there, with a value. Not that.

Second guess: maybe NEXTAUTH_URL_INTERNAL (a lesser-known NextAuth variable used behind proxies) was set to something malformed. I grepped .env.local for it. The line didn't exist at all. Not that either.

Two guesses, two misses. At that point I stopped inventing theories about .env.local and went looking for hard evidence instead — specifically, whatever code was actually constructing the URL that was failing. The build output is right there in .next/server/, so I grepped the compiled chunk for the literal pattern the stack trace implied:

grep -o 'new URL([^)]*' .next/server/chunks/662.js

Nineteen matches. All nineteen referenced exactly three identifiers: NEXTAUTH_URL, NEXTAUTH_URL_INTERNAL, and VERCEL_URL. That's not application code — that's next-auth/react itself. My own code doesn't construct a URL object anywhere near auth.

That grep was the actual turning point, and it's worth naming why it worked when guessing didn't. A .env file is a hypothesis space — you can stare at it and imagine a dozen ways a value could be wrong. A compiled bundle is not a hypothesis space; it's a record of what the code actually does. Every new URL(...) call in that chunk is a call some real module makes, using real variable names, and there's no interpretation involved in reading it. I didn't need to know anything about NextAuth internals going in — the grep told me exactly which three environment variables mattered and, implicitly, which package to go read next. Compiled output is one of the few artifacts in a JS project that can't be wrong about what it contains.

Why a client component can crash a build with an uncatchable error

Here's the part that made the try/catch in layout.tsx irrelevant: next-auth/react's SessionProvider module runs parseUrl(process.env.NEXTAUTH_URL ?? process.env.VERCEL_URL) at import time — at the top level of the module, not inside a function that gets called (and could be wrapped) later. By the time Providers even renders, the module has already executed and either succeeded or thrown.

parseUrl itself (from next-auth@4.24.15's utils/parse-url.js) is short:

function parseUrl(url) {
  var _url2;
  const defaultUrl = new URL("http://localhost:3000/api/auth");
  if (url && !url.startsWith("http")) {
    url = `https://${url}`;
  }
  const _url = new URL((_url2 = url) !== null && _url2 !== void 0 ? _url2 : defaultUrl);
  // ...
}

The bug hinges on one line: (_url2 = url) !== null && _url2 !== void 0 ? _url2 : defaultUrl. That's just url ?? defaultUrl written out — nullish coalescing. If url is undefined, this correctly falls back to defaultUrl. But if url is an empty string, "" is not null and not undefined, so the nullish check passes it straight through. And a few lines up, if (url && !url.startsWith("http")) also short-circuits on falsy, so an empty string skips the https:// prefix step too. The empty string sails through both guards unmodified and lands in new URL(""), which throws immediately: TypeError: Invalid URL, input: ''.

So somewhere, NEXTAUTH_URL was resolving to "" — not undefined, not missing, but an actual empty string. And because this throw happens during module evaluation inside a Client Component that every route imports through the root layout, it aborts static generation for every route where that import gets evaluated — with no try/catch in my own code anywhere near it, because the exception isn't inside a function call I made; it's inside a <script>-equivalent module body that runs as a side effect of importing next-auth/react.

This is a general property of import, not something specific to this library: importing a module runs its top-level code immediately, synchronously, before you get a reference to anything it exports. A try/catch you write can only guard the code between its braces — it has no reach into work a dependency chose to do the moment it was loaded. Singleton clients, feature-flag SDKs, analytics initializers, anything that reads process.env at the top of a file rather than inside an exported function, can all fail this same way: the crash happens on import, in a stack frame that belongs to someone else's package, at a point in the render tree your own error boundaries were never positioned to reach.

The number that confirmed it

The stack trace hit exactly 13 pages: /, /blog, four blog post pages, /compare, /faq, /how-it-works, and four /legal/* pages. Three routes in the app — /signup, /dashboard, /dashboard/team — were not affected, and all three had export const dynamic = "force-dynamic", meaning Next.js skips prerendering them at build time entirely. They never evaluate the layout's module tree during next build the same way, so they never hit the crash.

Thirteen static pages, thirteen failures, zero false positives among the dynamic ones. That's the kind of exact match that turns a hypothesis into a near-certainty before you've even fixed anything.

Where the empty string was coming from

NEXTAUTH_URL in .env.local had a real value. So why was the build reading ""?

Next.js loads env files in a specific priority order, and it's different depending on which command you run. For next build (which runs with NODE_ENV=production), the order is roughly:

.env.production.local
.env.local
.env.production
.env

next dev, meanwhile, prefers .env.development.local — a completely different file. That's exactly why npm run dev never showed the problem: it was never reading the file that had the bad value.

And sitting in my working directory, dated about a month earlier, was a leftover .env.production.local. I'd created it running vercel env pull .env.production.local --environment=production while debugging something else entirely. What I hadn't caught at the time: Vercel's CLI doesn't return the actual values for env vars marked "sensitive" via env pull — it writes them back as empty strings. I'd pulled a file full of blanks, moved on to the actual thing I was debugging, and never deleted it. Next.js then quietly gave that file top priority over my real .env.local on every production build, from that day forward, for everyone who ran next build locally.

And here's the part that let it hide for a month: .env.production.local matches the .env*.local pattern that ships in every default Next.js .gitignore, right alongside .env.local itself. It was never staged, never committed, never visible in a diff, never something a teammate or a past version of me could have flagged in review. It just sat on disk, correctly ignored by git for exactly the reason .env.local files are supposed to be ignored — because they're meant to hold real secrets — while accidentally also being an override file that Next.js would treat as authoritative the next time anyone ran a production build from that machine. The gitignore rule that protects secrets and the priority rule that resolves env files are two completely independent systems that happen to both key off the same filename convention, and nothing forces them to agree with your intentions at the same time.

The fix, once identified, took one command:

rm .env.production.local
npx next build

The first time I ran that rebuild, it still looked broken — until I noticed the .next/BUILD_ID timestamp didn't match anything I'd just done. I had two checkouts of the repo open (a worktree for an unrelated branch, and the main one), and the rebuild had run in the wrong directory against a .next cache that had nothing to do with the fix. Once I chained the cd and the build into a single command instead of relying on whichever shell happened to have focus, the real result came back clean: all 24 pages generated, zero errors, Environments: .env.local printed at the top of the build log where .env.production.local used to be listed first.

It's a small thing, but it's the same category of mistake as the bug itself — trusting which directory or which file you think you're operating in, instead of checking what the tool actually reports it used. The build log telling you which env files it loaded, in priority order, was there the whole time; I just hadn't been reading that line until this happened.

The lesson that generalizes

A few things about this are specific to Next.js and NextAuth, but the shape of the bug isn't:

next build and next dev don't read the same env files. This is documented, but it's the kind of documented-but-easy-to-forget fact that only bites you when a stale file happens to exist. If a bug only reproduces in one command and not the other, checking which .env* files each one actually loads should be an early step, not a last resort.

A tool that writes credentials back to disk can write blanks instead of failing loudly. vercel env pull succeeding with exit code 0 told me nothing about whether the values inside were real. A pull that returns empty strings for sensitive vars is functionally a silent failure wearing a success message. If you ever run a credential-pulling command for a one-off debugging session, treat the output file as radioactive — name it something that will never be picked up by convention (not .env.production.local), and delete it the moment you're done.

Two wrong guesses is a signal to stop guessing. Both of my first theories were reasonable and both were about the same file (.env.local) that turned out to be completely fine. The thing that actually worked was going one level closer to the truth: instead of theorizing about what might be in an env file, I grepped the compiled output for the literal code path that was throwing. Compiled output doesn't lie about what identifiers it references. If you find yourself on a second incorrect theory, that's usually the point to trade speculation for whatever raw artifact — a compiled bundle, a request log, a database row — can tell you definitively what actually ran.

A try/catch only catches what runs inside it. My layout's try/catch around getServerSession() was correct and necessary, but it gave me false confidence that env-var failures near auth were handled. A separate, unrelated code path — a third-party client component's module-level side effect — could still crash the same render with no relation to the code I'd defended. When a library does real work at import time rather than inside a function you call, no amount of wrapping your own call sites protects you from it.

None of this was a Next.js bug or a NextAuth bug. Both behaved exactly as documented. It was one leftover debugging artifact, two library behaviors that are individually reasonable, and a build tool that reads a different file than the one I was staring at. That combination is what took an afternoon to trace back to a single rm.

A short checklist, if this ever happens to you

If next build fails somewhere next dev doesn't, and the error points toward configuration rather than logic, this is roughly the order I'd check things in now:

ls -la .env* in the project root. Anything named .env.production.local, .env.production, or similar that you don't remember writing on purpose is suspect by default — delete it and rebuild before doing anything else.
If the build output already exists, grep the compiled chunks for the specific string in the error (a URL, a variable name, whatever's unique) before forming a third theory about source files you've already checked twice.
Treat any command that pulls secrets from a remote source (vercel env pull, aws secretsmanager get-secret-value, doppler secrets download, and so on) as something that can succeed with exit code 0 and still hand you garbage. Sensitive/encrypted values are the most likely category to come back blank, redacted, or truncated depending on the tool's permission model — check a value, don't just check the exit code.
If a third-party package throws during static generation, don't assume the crash originates in a function you called. Client components and providers can run real code — including new URL(...), fetch, or anything else — the moment they're imported, before your own try/catch blocks ever get a chance to run.

Every one of those steps would have gotten me to the answer faster than the two guesses I actually made.

Stripe said "Cancels". My dashboard said "Renews". The webhook returned 200 the whole time.

phi_blankslate — Wed, 23 Sep 2026 05:51:46 +0000

I run a small subscription SaaS on Next.js, Drizzle and Postgres, billed through Stripe. Before opening it up to real customers I did the most boring test there is: I became a customer in production. Real card, real trial, real Customer Portal. Then I canceled.

Stripe's side looked perfect. The subscription showed a clear "Cancels" badge with the end date. My own dashboard, the page my customers actually look at, still said:

Renews on

A hard refresh didn't change it. Both webhook deliveries for the cancellation had returned 200. My runtime logs showed zero errors. Every health signal I had was green, and the one thing a customer who just canceled wants to see, confirmation that they won't be charged again, was wrong.

This post is about what caused it, why my first theory was wrong, and the small set of habits I now use for any code that reads fields from a payment provider.

The code that "obviously" worked

Here's the relevant part of my webhook handler, before the fix. The handler doesn't trust the event payload. For every customer.subscription.* event it re-fetches the subscription from Stripe, so an old or out-of-order event can never overwrite newer state:

case "customer.subscription.created":
case "customer.subscription.updated":
case "customer.subscription.deleted": {
  const eventSub = event.data.object as Stripe.Subscription;
  await handleSubscriptionEvent(eventSub.id);
  break;
}

async function handleSubscriptionEvent(subscriptionId: string) {
  const stripe = getStripe();

  let sub: Stripe.Subscription;
  try {
    sub = await stripe.subscriptions.retrieve(subscriptionId);
  } catch (err) {
    if (isResourceMissingError(err)) {
      await deactivateSubscriptionByStripeId(subscriptionId);
      return;
    }
    throw err;
  }
  // ... then upsertSubscription(sub)

And the line that decided what the dashboard would show:

cancelAtPeriodEnd: sub.cancel_at_period_end,

The dashboard reads that boolean and renders one of two words:

{cancelAtPeriodEnd ? "Cancels" : "Renews"} on{" "}
{new Date(currentPeriodEnd).toLocaleDateString()}

If you've integrated Stripe Billing in the last few years, that line probably looks familiar. cancel_at_period_end is the field every tutorial, blog post and Stack Overflow answer about "show the user their subscription is ending" points at. The name even describes exactly what happened: the customer canceled, and the subscription will end at the end of the period.

It was false.

My first theory was wrong

Two webhooks came in for the cancellation. When a boolean is "stuck" and there are multiple events involved, the reflex is to blame ordering. My first hypothesis, written down in my notes at the time, was a race condition: the events arrive out of order, the older state wins, the flag gets overwritten.

It's a reasonable-sounding theory. It's also the kind of theory you can "fix" without ever confirming it. Add a timestamp check, add a lock, redeploy, cancel again, and if the dashboard happens to update you'll believe you fixed a race that never existed.

Two things stopped me from doing that.

First, the handler above already re-fetches the subscription from Stripe on every event. Out-of-order delivery shouldn't matter, because whichever event is processed last reads the current state, not the state embedded in the event. A race theory has to explain why the current state would still say "not canceling". It couldn't.

Second, I didn't have the evidence yet. So instead of patching, I went to get it.

Where the evidence actually was

My first stop was my hosting provider's runtime logs. They were gone. By the time I sat down to debug, the entries for the cancellation had already aged out of the log retention my plan includes. I hadn't thought about that window at all until I needed something outside it.

The fix for that was simple once I noticed it: the payment provider keeps its own event log, and it keeps it for longer. In Stripe Workbench, every webhook event is stored with its full payload, including previous_attributes, the diff Stripe computed for that change.

That diff answered the question in one screen. For the cancellation event:

previous_attributes:
  cancel_at:                    null  → <timestamp>
  canceled_at:                  null  → <timestamp>
  cancellation_details.reason:  null  → "cancellation_requested"

And cancel_at_period_end? Not in the diff at all. It was false before, and false after.

So Stripe was telling me, very precisely, what had changed. The customer's cancellation was recorded as a cancel_at timestamp. The field I was reading was never involved. No race. No lost update. A deterministic bug: I was reading a field that this subscription would never set.

Why `cancel_at_period_end` stayed false

The payload had one more field that explained everything:

billing_mode:
  type: "flexible"

Stripe's changelog for the Basil API release covers this. cancel_at_period_end is deprecated, and for subscriptions on the flexible billing mode, a "cancel at the end of the period" request is resolved immediately into a concrete cancel_at timestamp. The boolean stays false. The end date is the signal.

When I read it that way, it makes sense. "Cancel at period end" is a relative instruction. It means "whenever this period happens to end". A timestamp is an absolute one. If the platform resolves the relative instruction into an absolute date the moment you make the request, there's no reason to keep a boolean that says the same thing less precisely. From Stripe's side, nothing is broken. The data model moved, and my code was still reading the old one.

One detail I got wrong along the way and want to flag, because it's an easy mistake: my first draft of the code comment said this happens "when a customer cancels during a trial". That was the situation I happened to test, so I assumed it was the cause. It isn't. The condition is the billing mode, not the trial. An active, paying subscription on flexible billing behaves the same way. If I had shipped the comment as I first wrote it, the next person to read the code (probably me) would have believed the bug only affects trials and might have "optimized" the check away for paid plans.

The fix

The fix is a single expression:

// Stripe API Basil (2025-05-28 / 2025-07-30 changelog "cancel at enums"):
// cancel_at_period_end is deprecated. For billing_mode=flexible subscriptions a
// period-end cancel is resolved immediately into a cancel_at timestamp and
// cancel_at_period_end stays false (trial or not; confirmed against production
// payloads). Check both, so the old behaviour keeps working too.
cancelAtPeriodEnd: sub.cancel_at_period_end || sub.cancel_at != null,

(The real comment in my repo is in Japanese; this is a faithful translation.)

A few things about this I thought about before shipping it.

Why keep cancel_at_period_end at all? Backward compatibility. Older subscriptions or a future change to the API version I'm pinned to could still set the boolean. The || means either signal is enough. Removing the old check wouldn't make the code more correct, only more fragile.

Could cancel_at != null show "Cancels" to someone who never canceled? This was the real risk, so I listed every way cancel_at gets set in my setup: an explicit cancellation from the Portal or API, a cancellation date set manually in the Stripe dashboard, or a subscription schedule that ends in cancellation. All three genuinely mean "this subscription is going to end". I don't use subscription schedules. Failed-payment handling (dunning) moves the subscription through statuses like past_due rather than pre-setting cancel_at, per Stripe's docs. So in my setup, a non-null cancel_at means a real scheduled cancellation.

What does this value actually control? I grepped for every consumer of cancelAtPeriodEnd. There was exactly one: the "Cancels / Renews" label on the dashboard. It doesn't touch billing amounts, entitlements or usage limits. Stripe stops charging independently of what my app displays. So if this fix were somehow wrong, the worst case is a wrong word on a page, and the rollback is a single-commit revert. That's what let me ship it with confidence instead of agonizing over it.

A second bug hiding behind the first

While confirming the fix, I found something I'd never have looked for otherwise: my app was talking to Stripe in two different API versions at once.

My Stripe client was created like this:

stripeClient = new Stripe(secretKey, {
  // Follow the API version the current SDK recommends (unset = SDK default)
  typescript: true,
});

No apiVersion. So every retrieve() call used whatever version the installed SDK pins by default, which at the time was 2026-03-25.dahlia.

The webhook endpoint, on the other hand, was registered in the Stripe dashboard with its own API version, 2026-06-24.dahlia. Webhook payloads are rendered in the endpoint's version, not your SDK's.

That means the payload I was reading in Workbench, which I used to diagnose the bug, was not the same shape of data my code reads via retrieve(). In this particular case both versions are after the Basil change, so they agree about cancel_at. But it was luck, not design. If those two versions had straddled a breaking change, I could have looked at a payload, confirmed a field behaves a certain way, and shipped a fix that does nothing, because my code never sees that version of the object.

Pinning apiVersion explicitly so the SDK and the webhook endpoint agree is the permanent fix. I deliberately kept it out of this change, since it touches every Stripe call in the app, and a one-line billing fix is the wrong place to slip in a global behavior change. It's tracked as its own task.

One thing I left alone on purpose

Look at the dashboard snippet again:

{cancelAtPeriodEnd ? "Cancels" : "Renews"} on{" "}
{new Date(currentPeriodEnd).toLocaleDateString()}

The fix changed which word appears. It didn't change which date appears. The date still comes from the current period end, not from cancel_at.

For the cancellation I tested, those are the same moment. I checked the payload: cancel_at, the trial end and the current period end all held the same value, which is exactly what you'd expect from "cancel at the end of the period". But cancel_at can in principle be any timestamp. If someone set a cancellation for the middle of a period, say by picking a custom date in the Stripe dashboard, my page would say "Cancels on" followed by the wrong date.

I noticed this during review and deliberately did not fix it in the same change. In my setup the only person who can set an arbitrary cancellation date is me, from the Stripe dashboard. Customers can only cancel through the Portal, which resolves to the period end. The right fix is to store cancel_at as its own column and render that, which means a schema change, a migration and a second change to billing code. That's a separate, reviewable piece of work, not something to bundle into a one-line hotfix just because I happened to be in the file.

I think this is the part of billing work that's easy to get wrong in the other direction. Once you've found one bug, it's tempting to "clean up" everything nearby while you're there. On a money path, every extra line you touch is another thing a reviewer has to reason about and another thing a revert has to undo. I wrote the gap down as a known limitation with the exact condition that would trigger it, and moved on.

How I verified it without creating a new subscription

The obvious way to test this fix would be to run the whole flow again: new checkout, new trial, cancel in the Portal, check the dashboard. That creates another real subscription in production just to test a display label.

Stripe Workbench has a better tool: Resend. You can re-deliver any past webhook event to your endpoint. Because my handler re-fetches the subscription instead of trusting the payload, resending the original cancellation event after deploying the fix simply makes my code read the current subscription again and upsert it. No new charge, no new customer, nothing to clean up afterwards.

So the verification was:

Deploy the fix, in a commit that contained only the webhook change, so it could be reverted on its own
Resend the original customer.subscription.updated event from Workbench
Confirm the endpoint returned 200 on the new deployment
Reload the dashboard

It said "Cancels on ".

That step also closed the last open question I had: whether retrieve(), on the SDK's older API version, actually returns cancel_at populated. I had only inferred it before. The label flipping proved it.

What I took away from this

None of this is specific to Stripe, which is why I wanted to write it up. Here's what I've changed in how I work with any third-party API that controls money or access.

1. A 200 is not the same as a correct result

Every monitoring signal I had was about delivery: status codes, error counts, exceptions. None of them were about meaning. The webhook was received, processed and stored successfully. It just stored the wrong answer. If your only alerting is "did it throw", a field-mapping bug is invisible by design. The only thing that caught this was a human doing the end-to-end flow and looking at the result.

2. Get the payload before you form a theory

"Race condition" felt like a diagnosis, but it was a guess shaped like a diagnosis. The real answer was sitting in previous_attributes the whole time. My rule now: for any "the state is wrong" bug involving a webhook, I don't write a line of fix until I've seen the actual payload and the actual diff for the event in question.

3. Use the provider's logs, not just yours

Your hosting logs have a retention window, and you probably don't know how long it is until you need something outside it. Stripe, and most serious payment and identity providers, keep a full, searchable event history with payloads. For money-path debugging, I now treat the provider's event log as the primary source and my own logs as supplementary.

4. Deprecated fields don't error. They go quiet.

cancel_at_period_end still exists in the type definitions. It still comes back in the response. It still has a perfectly valid value. It's just no longer the field that carries the information. There was no warning, no exception and no undefined to trip over. A field that is present, typed and plausible but no longer authoritative is the worst kind of deprecation, and the only defense I know of is reading the changelog for any field that drives something a user sees.

5. Know which API version produced the data you're looking at

Payloads in the dashboard, payloads your endpoint receives, and objects your SDK retrieves can each be rendered in different API versions. When you diagnose from one and fix code that reads another, make sure they match, or pin them so they can't drift apart.

6. Size the blast radius before you ship

The single most useful thing I did before deploying was grep for every consumer of the field I was changing. Finding exactly one, a display label, turned a nervous billing-code change into a low-risk one with a trivial rollback. If I'd found it feeding into plan limits or charge amounts, the right move would have been a much slower rollout.

7. Test the thing the customer sees

I'd tested my webhook handler. I hadn't tested the sentence "your subscription will end on this date" as a customer would read it, right after canceling, in production. That sentence is the whole point of the feature. It's also the one piece nothing in my automated checks would ever look at.

If you use `cancel_at_period_end` today

It's worth a five-minute check:

Search your codebase for cancel_at_period_end
For each hit, ask whether that code also looks at cancel_at
Check which API version your SDK client uses, and which version your webhook endpoint is registered with. If you never set apiVersion, you're on the SDK default, which changes when you upgrade the package
If you can, cancel a test subscription and look at the event's previous_attributes in the dashboard to see which fields actually change on your account

If the answers surprise you, you might have a dashboard telling your customers the opposite of what's actually going to happen, while every webhook returns 200.

I'm curious whether others have hit this. Did the Basil change catch you too, or did you move to cancel_at before it mattered?

How I built an AI code reviewer that knows when to shut up

phi_blankslate — Fri, 18 Sep 2026 08:43:34 +0000

Every AI code reviewer I tried had the same problem: it wouldn't stop talking.
Rename this. Add a comment here. Consider extracting that. By the third file
you've stopped reading, and a tool you've stopped reading is worse than no
tool — it's a tool that will hide a real bug from you inside a wall of
suggestions.

So I built one with the opposite rule: say nothing unless you found something
worth saying. That sounds like a prompt engineering problem. It isn't. The
model will happily agree to be concise and then hand you fourteen findings
anyway. Every constraint that actually held up in production is a constraint
I enforce in application code, after the model has already spoken.

Here's what that looks like, including three bugs I only found by pointing
the thing at real pull requests and a real credit card.

The problem: nitpick fatigue is a trust problem, not a UX problem

A reviewer you've muted is worse than no reviewer, because now there's a wall
of suggestions for a real bug to hide behind. This matters most for solo
developers and small teams — the people who don't already have an enterprise
code-review bundle sitting on top of their existing tools. If the review
output is noisy, they turn it off in week one and never come back.

Architecture in one line

GitHub webhook → diff fetch → Claude → structured findings → ranker → inline comments

Every stage after "Claude" exists to decide what not to show you.

Structured output, then rank it yourself

The model returns JSON findings, each with { path, line, severity, message },
where severity is one of four values: BUG | WARN | NIT | PRAISE. The
severity string coming back from the model is validated against that
whitelist — an invalid value gets the finding dropped rather than trusted.
Then everything is stable-sorted by severity:

const severityRank: Record<ReviewComment["severity"], number> = {
  BUG: 0,
  WARN: 1,
  NIT: 2,
  PRAISE: 3,
};
const rankedComments = sanitizedComments
  .map((comment, index) => ({ comment, index }))
  .sort((a, b) => {
    const rankDiff = severityRank[a.comment.severity] - severityRank[b.comment.severity];
    if (rankDiff !== 0) return rankDiff;
    return a.index - b.index; // same-severity ties keep the model's own order
  })
  .map((entry) => entry.comment);

Stable sort matters here: within the same severity, findings keep the order
the model produced them in, instead of being silently reshuffled.

The cap is a constant, not a request

const comments = rankedComments.slice(0, MAX_COMMENTS);
const suppressedCount = sanitizedComments.length - comments.length;

MAX_COMMENTS is a constant, applied by slice after the sort — not a line
in the prompt asking the model to "please be concise." A prompt is a request
the model can ignore on a bad day; a slice() cannot.

What gets cut isn't dropped silently. suppressedCount flows into the review
body:

body += `🔇 ${result.suppressedCount} lower-priority remark${result.suppressedCount !== 1 ? "s" : ""} suppressed to keep this review focused.\n\n`;

Why disclose the count instead of just trimming quietly? Because "quiet
reviewer" and "reviewer that missed it" look identical from the outside
unless something tells you which one you're looking at.

Fair pushback on this design, and I don't have a clean answer: on a PR with
more than 8 genuine bugs, the cap works against you. Sorting bugs to the
front means you at least see the worst of it first, but "the cap doesn't
hide real bugs" is not a guarantee I'm willing to write — only that severity
ordering makes it less likely.

PRAISE is not decoration

There's a fourth severity that isn't a problem at all — a slot reserved for
"this was a good change." A review that's only ever negative gets the same
treatment as a chatty one: people stop opening it.

Position anchoring, and the fallback that keeps a finding alive

GitHub's inline PR comments only land if the position matches the actual
diff hunk. If a finding's line doesn't anchor, the naive move is to drop it.
Instead, the handler falls back to a single top-level comment that lists
everything, formatted findings included:

try {
  await createReview(/* ...inline comments... */);
} catch (reviewError) {
  console.error("Inline review failed, falling back to issue comment:", reviewError);
  await postPRComment(octokit, owner, repo, prNumber, fallbackCommentBody(reviewResult));
}

A formatting mismatch shouldn't be able to delete a finding.

Bug #1 — the webhook that succeeded and failed at the same time

GitHub wants a 2XX response within roughly 10 seconds of a webhook delivery.
Generating a review — fetch the diff, call the model, post the comments —
routinely takes longer than that. Doing all of it synchronously meant GitHub
logged the delivery as failed while my function kept running and posted the
comments anyway. The delivery log said "failed." The PR said otherwise. Both
were technically correct, which made it maddening to debug.

The fix: verify the signature, return 200 immediately, and do the actual
work in the background.

case "pull_request":
  waitUntil(
    handlePullRequestEvent(payload).catch((error) => {
      console.error("PR review background processing failed:", { deliveryId }, error);
    })
  );
  break;

Bug #2 — the silent fail that going async created

Async fixed the timeout problem and introduced a worse one: if the
background work dies partway through, the review row just sits at
"pending" forever. Not visible on the dashboard as an error. Not counted
against quota. GitHub already has its 200. Nothing anywhere tells you a
review didn't happen — that's the definition of a silent failure.

The fix was to stop depending on platform defaults and make the ceiling
explicit:

// Give the background work (PR file fetch + Claude review + GitHub post)
// enough time. Leaving this unset silently inherits Vercel's default,
// and on timeout the review sits at "pending" with no way to detect it.
export const maxDuration = 120;

Going async doesn't remove failure, it changes what failure looks like.
Anything that runs outside the request/response cycle needs its own
explicit way of surfacing "this didn't finish" — a timeout, a status you
can query, something. Silent pending isn't good enough.

Bug #3 — the subscription that said "Renews" after being cancelled

This one wasn't a code review bug, it was a billing bug, and I only found it
because I ran my own Stripe checkout on the live account. Cancelled a trial
from the customer portal. Stripe's own dashboard confirmed "cancels
[date]." My app's dashboard kept saying "Renews."

The webhook handler was trusting cancel_at_period_end as the single source
of truth for "is this cancelling":

cancelAtPeriodEnd: sub.cancel_at_period_end,  // before the fix

Pulling the actual webhook payload in the Stripe dashboard showed the real
shape of the event: cancel_at_period_end stayed false through the whole
cancellation, while cancel_at flipped from null to a real timestamp.
Current Stripe subscription behavior resolves an end-of-period cancellation
directly to a cancel_at timestamp rather than only flipping the boolean.
The fix reads both:

// cancel_at_period_end alone isn't reliable here — cancellation can resolve
// straight to a cancel_at timestamp instead. Check both, keep the boolean
// path for backward compatibility.
cancelAtPeriodEnd: sub.cancel_at_period_end || sub.cancel_at != null,

Two things I took from this: don't trust a single boolean field to represent
a state transition without checking the real payload first, and billing
paths get tested with a real card, not a happy-path assumption about what
the API returns. Six lines to fix, but shipped as-is it would have told
every cancelling customer a lie about their own subscription.

Try it

DevReview reviews GitHub pull requests and ranks findings BUG → WARN → NIT →
PRAISE, hard-capped at 8 comments per review with the suppressed count
disclosed rather than hidden. Free tier is $0 forever — 15 reviews/month on
one repo, no card involved. Pro is $9/user/month with a 14-day trial (card
required at checkout, first charge on day 15).

https://getdevreview.com

I'd genuinely like to know where the 8-comment cap is the wrong call —
tell me if you try it.

DEV Community: phi_blankslate

My Stripe idempotency key was correct. It just stopped working after 24 hours.

The feature

Layer one: make the database the gate

Layer two: the idempotency key

The rollback that made it dangerous

The 24-hour window

The fix: check Stripe's state, not your own memory

A stronger anchor: record the transaction ID

The related trap I found one pass later

What I would tell myself before writing this

The uncomfortable part

drizzle-kit generated a DROP TABLE for a table that was still in production. drizzle-kit wasn't the problem.

The setup

How the drift happened

More than a month later: drizzle-kit generate

Why the diff looks like that

What each statement would have done

What I actually did

Why the side door existed in the first place

The check I already had, and why it wasn't enough

What I do now

The general lesson

A stray .env file broke exactly 13 pages of my Next.js build — and dev mode never saw it

The setup

Two wrong guesses, then I stopped guessing

Why a client component can crash a build with an uncatchable error

The number that confirmed it

Where the empty string was coming from

The lesson that generalizes

A short checklist, if this ever happens to you

Stripe said "Cancels". My dashboard said "Renews". The webhook returned 200 the whole time.

The code that "obviously" worked

My first theory was wrong

Where the evidence actually was

Why cancel_at_period_end stayed false

The fix

A second bug hiding behind the first

One thing I left alone on purpose

How I verified it without creating a new subscription

What I took away from this

1. A 200 is not the same as a correct result

2. Get the payload before you form a theory

3. Use the provider's logs, not just yours

4. Deprecated fields don't error. They go quiet.

5. Know which API version produced the data you're looking at

6. Size the blast radius before you ship

7. Test the thing the customer sees

If you use cancel_at_period_end today

How I built an AI code reviewer that knows when to shut up

The problem: nitpick fatigue is a trust problem, not a UX problem

Architecture in one line

Structured output, then rank it yourself

The cap is a constant, not a request

PRAISE is not decoration

Position anchoring, and the fallback that keeps a finding alive

Bug #1 — the webhook that succeeded and failed at the same time

Bug #2 — the silent fail that going async created

Bug #3 — the subscription that said "Renews" after being cancelled

Try it

More than a month later: `drizzle-kit generate`

Why `cancel_at_period_end` stayed false

If you use `cancel_at_period_end` today