DEV Community: phi_blankslate

I almost killed a project because my test harness was measuring its own bug

phi_blankslate — Sat, 31 Oct 2026 22:36:09 +0000

I was one command away from spending real API money on an experiment whose answer would have been wrong — and wrong in the most dangerous direction: it would have told me the thing I was testing worked, for the wrong reason, or failed for a reason that had nothing to do with it.

What saved me was boring. Before running the paid version, I ran the free version and actually read the raw data with my own eyes. This post is about what I found there, why measurement bugs tend to lie in your favor, and the rule I now follow before any expensive evaluation.

The question I was trying to answer

The project was a small accessibility tool. The pipeline looked like this:

Load a real web page in a headless browser (Playwright).
Run axe-core on it to detect accessibility violations — missing button labels, images without alt text, invalid ARIA attributes, that kind of thing.
For each violation, hand the offending HTML element to an LLM and ask it for a fix in a strict format:

BEFORE:
```html
<the element as it is now>
```
AFTER:
```html
<the fixed element>
```
EXPLANATION: ...

(The code comments you'll see below are in Japanese — that's how they are in the actual repo, and I've kept them as-is rather than rewrite code for the post.)

Score the result.

The whole project hinged on one kill question: are the AI's fix suggestions actually usable? If not, there was no product. So I wrote a verification harness that would run the pipeline over a list of real sites and classify every response.

The classifier was strict on purpose. A response was only counted as a valid, applicable fix if it passed a chain of checks, in this order:

// 判定順序は仕様通り: api_error -> no_text -> truncated -> malformed -> unresolved -> noop -> before_mismatch -> source_truncated -> valid_applicable

The two checks that matter for this story are noop and before_mismatch:

if (normalizeHtml(parsed.after) === normalizeHtml(parsed.before)) return 'noop';
if (normalizeHtml(parsed.before) !== normalizeHtml(nodeHtml)) return 'before_mismatch';

In plain English: the model's BEFORE block has to match the element I gave it. That is how I make sure the fix can actually be applied to the page as a find-and-replace patch, rather than being a hallucinated rewrite of something that doesn't exist.

That check felt rigorous. It turned out to be the exact place where the harness lied to me.

The free run

The paid run needed an API key that I didn't have yet. While waiting, I added a --dry-run flag:

//   node k2-verify.js --dry-run  APIを一切呼ばずscanUrlの配線確認のみ

It runs the browser and axe-core against every site, builds every input the LLM would have received, and writes them all to disk — but never calls the API. Zero cost. The point was just to confirm the wiring.

I almost didn't read the output. The run completed, the summary table looked plausible, and the natural next step was "wait for the key." Instead I opened the raw JSON and read the actual HTML snippets I was about to send to the model.

Many of them were broken.

Bug 1: axe-core's `node.html` is not the element

I had assumed that violation.nodes[i].html — the HTML string axe-core attaches to every violation — was the element's markup. It's not. It's a summary of the element, built for humans reading a report.

I didn't trust my memory about how it was built, so I opened node_modules/axe-core/axe.js and read the function that produces it. In the version I had installed (4.12.1), _getElementSource takes a maxLength that defaults to 300 and an attrLimit that defaults to 20:

function _getElementSource(node) {
  var _ref24 = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : {}, _ref24$maxLength = _ref24.maxLength, maxLength = _ref24$maxLength === void 0 ? 300 : _ref24$maxLength, _ref24$attrLimit = _ref24.attrLimit, attrLimit = _ref24$attrLimit === void 0 ? 20 : _ref24$attrLimit;
  ...
  var deepStr = getOuterHtml(node);
  if (deepStr.length > maxLength) {
    return getTruncatedElementSource(node, {
      maxLength: maxLength,

and the attribute truncation is this:

function truncate(str, attrLimit) {
  return str.length <= attrLimit ? str : str.substring(0, attrLimit) + '...';
}

So when an element's outerHTML is longer than 300 characters, you don't get the element. You get its opening tag only — children gone, closing tag gone — with long attribute values cut to 20 characters plus ....

That's a completely reasonable design for a violations report. It's a terrible input for "please write a patch for this exact markup."

Why this bug points toward "pass"

Here's the part that still makes me uncomfortable.

When you hand an LLM a truncated, unclosed element and ask for BEFORE and AFTER, a well-behaved model copies what you gave it into BEFORE. Truncated attribute and all. Then my strict classifier compares BEFORE to the input I sent — the same truncated string — and they match.

before_mismatch passes. The response gets counted as an applicable fix.

But it isn't applicable. The real page doesn't contain <a href="https://example.co..."> with no closing tag. Nobody can find-and-replace that into a real codebase. The "patch" is a patch against a string that only exists inside my harness.

My most rigorous check was rigorous against the wrong ground truth. And because the error made the model look better, I would never have gone looking for it after a good-looking result. I would have celebrated.

The detection trap

My first instinct for detecting truncated inputs was to search for the ... marker. That turned out to be wrong in both directions:

False positives: real page text contains .... A heading like <h3>Be Bold...</h3> or an attribute like title="..." isn't truncation, it's content.
False negatives: if every attribute on an element is short, the opening-tag-only fallback leaves no marker at all. A <html lang="en"> with its entire document body silently dropped looks perfectly clean.

The reliable signal ended up being structural: a non-void element whose snippet has no closing tag has lost its children. A string search for a marker was never going to be enough, and I only found that out because I read the library source instead of guessing what it "probably" does.

The fix: re-fetch the real element

axe-core gives you a target array — a selector path to the element. So after axe runs, and before the browser context closes, I go back to the live page and ask for the real outerHTML:

async function refetchOuterHtml(page, target) {
  try {
    const locator = buildLocatorForTarget(page, target);
    const html = await locator.first().evaluate((el) => el.outerHTML);
    if (typeof html !== 'string') {
      throw new Error('evaluate did not return a string (outerHTML unavailable)');
    }
    return { ok: true, html, error: null };
  } catch (err) {
    return { ok: false, html: null, error: err.message };
  }
}

I still need some size cap for a prompt, so I wrote my own truncation — one that keeps the opening tag fully intact, says explicitly what was removed, and keeps the element closed:

const marker = `\n<!-- rammendo: children omitted (${omittedChars} chars) -->\n`;
const closing = tagName ? `</${tagName}>` : '';
return { html: `${openingTag}${marker}${closing}`, childrenOmitted: true };

The difference from the library's version isn't "truncate vs. don't truncate." It's that this truncation is declared. The model can see that children were omitted, the result record carries childrenOmitted: true, and the scorer can treat it accordingly instead of mistaking it for the full element.

And crucially: if the re-fetch fails, I don't quietly fall back to axe's string. I fall back loudly:

console.warn(
  `[rammendo] outerHTML再取得失敗 (rule=${violation.id}, url=${pageUrl}): ${refetch.error} — axeのnode.htmlにフォールバックします`
);
node.__rammendo = {
  html: node.html,
  sourceKind: 'axe_fallback',
  ...

Every record carries a sourceKind, and the summary report has a row that counts axe_fallback and prints a large warning block if it's anything other than zero. A silent fallback would have recreated the exact lie I had just removed, the first time a selector failed to resolve.

Bug 2: I was counting other people's bugs as my customer's

The second problem showed up in the same free run, in a different column.

One site had a lot of ARIA violations — far more than the rest. Reading the raw records, almost all of them were inside an embedded third-party booking widget. axe-core scans through iframes by default, which is the right default for an audit ("your users experience this page, including the widget"). It's the wrong default for my question, which was "can the AI fix the site owner's code?" The site owner can't edit markup that lives inside someone else's iframe.

So that site's numbers were inflated by violations nobody on that site could fix, and any fix the LLM wrote for them would be a fix to code nobody could apply.

The tempting fix is a heuristic: "if the class name contains the vendor's name, skip it." I explicitly didn't do that. Class names are guesses. Instead I used two mechanical signals:

axe's target array is longer than one entry when the element is inside a frame. The leading entries are the frame path.
For the outermost frame, read its actual src, resolve it against the page URL, and compare origins.

const thirdParty = iframeOrigin !== pageOrigin;
return { inIframe: true, framePath, iframeSrc, iframeOrigin, pageOrigin, thirdParty, originNote: null };

And when the signal is missing — an iframe with no src, or about:blank, or a srcdoc — I refuse to guess:

originNote = 'iframeにsrc属性が無いか、srcdoc/about:blank等の非URL値。推測でthird party判定はしない';

Third-party violations aren't deleted. They're reported in their own section, with the provider's origin, as "this is real, but it isn't in your code." Hiding them would be its own kind of dishonesty; mixing them into the fixable count was the other kind.

Why measurement bugs lean toward "pass"

Both bugs share a shape, and it's the reason I'm writing this up.

A bug in the thing you're testing usually makes results worse, and bad results get investigated. A bug in the instrument can make results better, and good results almost never get investigated. Nobody reruns an experiment that came out the way they hoped.

Bug 1 is the clean example. The strict "BEFORE must match the input" check is a self-consistency check. If the input is wrong, self-consistency still holds — the model is perfectly consistent with garbage. Every check that compares an output to the harness's own input inherits whatever is wrong with the input.

Bug 2 points the other way — inflated failure counts — but it has the same root: the instrument was answering a slightly different question from the one I asked. axe-core answers "what's wrong with this page as rendered?" I needed "what's wrong with this site owner's code?" The two differ by exactly the stuff inside third-party frames.

In both cases, if I had gone straight to the paid run, I would have gotten a number, and the number would have been about my harness rather than about the AI.

A quick checklist for "is my harness lying?"

After this, I wrote down the questions I now ask before I trust any number an eval harness produces. They're cheap, and most of them take less time than reading this section.

Where does each input come from, exactly? Not "from the scanner" — which field, produced by which function, with which defaults. If the answer involves a library, I want to have seen the line of code. In this case the answer was "a display string with a 300-character cap," and that one fact changed everything downstream.

What does a perfect score look like if the model does nothing clever at all? If a model that simply echoes its input back would pass a check, that check is measuring the harness, not the model. My BEFORE comparison failed this test: an echo passes it by construction. It's still a useful check — it rejects hallucinated rewrites — but it can't tell you whether the input was right.

What fraction of records took a fallback path? If the answer isn't printed in the summary, I don't know it, and "I don't know" tends to quietly become "zero" in my head. Now it's a row in the report with a warning attached.

Is my denominator the population I care about? The third-party iframe problem was a denominator problem. The scanner's population was "everything rendered on the page"; mine was "everything the site owner can edit." Counting the wrong population produces a precise answer to a question nobody asked.

Would I notice if this check broke silently? For each check I ask what the report would look like if it always returned "pass." If the honest answer is "about the same, maybe slightly better," that check needs a test of its own — a deliberately broken input that it has to reject.

None of this requires a framework. It's a short list in a markdown file next to the harness, and I read it before every paid run.

What I do now

These are the rules I took away. None of them are novel; I just hadn't been following them.

1. Before any paid evaluation, do a free run and read the raw inputs.
Not the summary table. The actual payloads that would have gone to the model. A dry-run mode costs almost nothing to add, and it's the only stage where the inputs exist without results to bias how you read them.

2. Ask which direction the instrument would lie.
For every check, I now ask: "if this check is broken, does it fail too much or pass too much?" Checks that would pass too much get extra scrutiny, because nothing downstream will catch them. Self-consistency checks — output compared to the harness's own input — are the first suspects.

3. Read the library source for any data transformation you depend on.
"axe gives you the element's HTML" was a belief, not a fact. Opening node_modules and finding maxLength and attrLimit took a few minutes. Anything a library does to your data — truncation, normalization, summarizing, deduplication — is worth confirming in code rather than in memory or docs.

4. Make every fallback visible and countable.
If the good path fails and you fall back to a worse source, tag the record and count it in the report. A fallback that can't be seen in the summary is just the original bug with extra steps.

5. Separate "what the tool reports" from "what you're actually asking."
A scanner's scope (the rendered page, including iframes) and your question's scope (the code a specific person can change) are often slightly different. Write the question down explicitly, then check whether the instrument's default matches it.

6. When a signal is missing, record that it's missing.
For the iframe check, "no src" doesn't mean first-party or third-party. It means unknown, and the record says so in a note. Guessing would have been quieter and worse.

The uncomfortable bit

The scariest part of this isn't that the harness had bugs. Every harness has bugs. It's that I was about to use the result to make a go/no-go decision about the whole project.

If the bugs had pushed the numbers down, I might have concluded "AI fixes aren't good enough" and dropped an idea because of a problem in my own wiring that took an afternoon to fix. If they had pushed the numbers up, I would have built on a foundation that wasn't there. Either way, the decision would have been made by my instrument, not by the thing I thought I was measuring.

The free run didn't make the project succeed. It made the eventual measurement mean what I wanted it to mean. That's a much smaller claim, and it's the one I actually care about now.

If you're building an eval harness for an LLM feature — especially one where the model's output gets compared to the input you constructed — I'd be curious how you guard against the same thing. Do you read raw payloads before every run, or do you have checks that catch a broken input automatically?

How We Secure DevReview (October 2026)

phi_blankslate — Sun, 25 Oct 2026 08:37:42 +0000

Originally published on the DevReview blog.

DevReview reads your pull requests. That means you are trusting us with code that isn't public yet. This post lays out what we do to deserve that trust, what outside scanners say about us, and what we have not finished.

This is our own write-up, not an audit. Scores are dated because they change.

What outside scanners say (measured 2026-10-24)

Qualys SSL Labs: A+ on one endpoint, A on the other. Our site is served from two edge addresses; we report both rather than the better one. Run the test yourself.
Mozilla HTTP Observatory: B (75), 11 of 12 checks passed. The one we lose points on is Content Security Policy. Our CSP currently runs in Report-Only mode: browsers report violations to us but do not block them yet. We are collecting reports first so that enforcing it doesn't break checkout or sign-in. We plan to switch to enforcement in early November 2026 and will update this post when we do. Run the test yourself.

Your code

We don't keep your code. DevReview fetches the diff it needs to review, sends it to the model, and posts comments. We don't store the diff, and we don't log the model's raw output.
Old data is deleted on a schedule. A daily job removes installations that were never completed after 30 days, and error details from failed reviews after 90 days. We checked the first scheduled run in production on 2026-10-24.

What this means for you: if you uninstall, there isn't an archive of your source code sitting on our side.

The doors into our servers

Webhooks are verified before anything else. GitHub and Stripe webhooks are checked for a valid signature first. Bodies over a fixed size (2 MB for GitHub, 1 MB for Stripe) are rejected with 413 before we parse them.
Errors don't leak internals. If something fails, you get a generic message. Stack traces and database errors stay in our logs.
HTTPS only. HSTS is set for two years.
New free accounts get a short wait. GitHub accounts younger than 7 days can't use the free plan, which keeps throwaway accounts from burning review capacity.
Referral codes are random. They come from a cryptographic random generator, not a counter you could guess.

How we run things behind the scenes

Two-step sign-in on every admin account we use to run DevReview (hosting, database, payments, domain, email, source code). One provider (our domain registrar) only offers emailed codes rather than an authenticator app.
Email that can't easily be spoofed. SPF, DKIM and DMARC all pass, and DMARC is set to quarantine mail that fails.
Breach monitoring for our domain via Have I Been Pwned.
Secret scanning before every push. A gitleaks hook blocks a push if it finds something that looks like a key.
Production dependencies: npm audit reports 0 known vulnerabilities as of 2026-10-22.
We practiced restoring the database. Our database keeps 6 hours of point-in-time history, and we ran a restore drill on a copy rather than trusting the docs.
A written first-30-minutes incident card, plus a ledger of every key we hold and how to rotate it.

What is not done yet

We'd rather say this plainly than have you find it:

CSP is Report-Only, as explained above.
Per-user rate limits and re-authentication for sensitive actions are built but not shipped yet. They're waiting on a final review.
CAA DNS records aren't set, because our DNS provider doesn't support them. We're looking at options.

Tips for your side

Install DevReview only on the repositories you want reviewed, not "All repositories".
Turn on two-factor authentication on your GitHub account if you haven't.
If you leave, uninstall the GitHub App. That stops all access immediately.

Found something?

Please tell us. Our security.txt and security page explain how to report a vulnerability. We read every report.

Stripe marks a subscription "active" before anyone has paid. Here's how I tested a reward that has to wait for real money.

phi_blankslate — Thu, 22 Oct 2026 01:09:06 +0000

I added a referral program to my SaaS. The rule sounds simple: when someone you referred becomes a paying annual customer, you get an account credit. Not when they sign up, not when they start a trial, but when money has actually moved.

Writing that logic took an afternoon. Proving it worked took much longer, and most of that time went into one uncomfortable fact: the only event that matters, "the referred customer paid", happens at the end of a trial, on an annual plan, and you only get one real shot at it per customer. The next natural billing event for that subscription is a year away.

This post is about how I tested that path without charging a real card, without creating fake accounts on any platform, and without touching my production database. The tools were Stripe Test Clocks and a disposable Postgres branch. Along the way the test surfaced two things I would not have found by reading the code: a $0 trial invoice counts as paid, and on recent Stripe API versions the invoice's subscription field is simply null.

The stack is Next.js on Vercel, Drizzle, and Neon Postgres, but the testing approach doesn't depend on any of that.

The bug that shaped the design: "active" is not "paid"

My first version of the reward check looked at the subscription status. If it was active and on the annual price, the referrer got their credit.

A security review caught the problem before it shipped. When a trial ends, Stripe moves the subscription out of trialing and creates the first real invoice. The customer.subscription.updated event with status: "active" can arrive before the payment on that invoice has succeeded. If the card is declined, the subscription later goes to past_due, but by then my code had already handed out a credit for a payment that never happened.

So the rule changed from "the subscription is active" to "the subscription is active and its latest invoice is paid and the amount paid is greater than zero". The trigger also moved: the main entry point is now the invoice.paid webhook, with customer.subscription.updated kept only as a fallback.

Here is the eligibility check as it exists in the code today (comments translated from my original notes):

export async function rewardReferralIfEligible(stripeSubscriptionId: string): Promise<void> {
  const [referral] = await db
    .select()
    .from(referrals)
    .where(eq(referrals.stripeSubscriptionId, stripeSubscriptionId))
    .limit(1);
  if (!referral || referral.status !== "pending") return;

  // Primary idempotency key: if a transaction ID is already recorded, the credit was granted.
  // Confirm "rewarded" without calling Stripe.
  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;
  }

  const stripe = getStripe();

  // Re-check eligibility against Stripe's actual state. Don't trust the caller's pre-filtering.
  const sub = await stripe.subscriptions.retrieve(stripeSubscriptionId, {
    expand: ["latest_invoice"],
  });
  if (sub.status !== "active") return;
  const priceId = sub.items.data[0]?.price.id ?? null;
  if (!priceId || priceId !== PLAN_PRICE_IDS.pro_annual()) return;
  const latestInvoice =
    typeof sub.latest_invoice === "object" && sub.latest_invoice !== null
      ? (sub.latest_invoice as Stripe.Invoice)
      : null;
  if (!latestInvoice || latestInvoice.status !== "paid" || (latestInvoice.amount_paid ?? 0) <= 0) {
    return;
  }

  // Compare-and-swap: only one process can move pending -> rewarded.
  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;

  // ... resolve the referrer's Stripe customer, check for an existing credit, grant it
}

Every condition in that function is a claim about how Stripe behaves at the end of a trial. I had read the docs, and a reviewer had read the code. Neither of those is evidence. I needed to watch a trial end.

Why the obvious test plans didn't work

Before I found a setup I liked, I talked myself out of three others. Each failure taught me something about what the test actually needed.

Plan 1: subscribe for real in production, then cancel during the trial. I had used this trick before to test checkout, because cancelling inside the trial costs nothing. But it tests nothing here. The reward only fires on invoice.paid after a successful charge. Cancel during the trial and the code under test never runs. A free test of the wrong path is not a cheap test of the right one.

Plan 2: subscribe for real, but give the test customer a balance credit large enough to cover almost the whole invoice. This looked clever until I reread my own guard: amount_paid > 0. If the credit covers the full amount, Stripe records the invoice as paid with nothing actually charged, and my code correctly refuses to reward it. To get through the guard I would have had to leave a small remainder on the card. That's workable, but it's a real charge on a real card in a real account, and it still runs against production data.

Plan 3, the one that actually killed the real-money approach: the referred customer has to be a different person. My self-referral guard rejects a referral when the referrer and the referred user share a user ID or an email address. That is the point of the guard. To test the happy path end to end in production, I needed a second account that signs in through the same OAuth provider my app uses. I run exactly one account on that platform on purpose. Creating a second account to test my own app is precisely the kind of thing that gets accounts suspended, and I wasn't going to trade that risk for a test.

Once all three were off the table, the shape of the right test was clear. I didn't need a real person or a real card. I needed Stripe to believe that a trial had ended and a payment had succeeded, and I needed my database to contain the rows that would exist if a real referral had happened. Both of those can be faked safely, as long as the fakes are fenced off from everything real.

The setup: Stripe test mode, a Test Clock, and a throwaway database branch

The test environment had three pieces:

Stripe test mode. Customers, subscriptions, and invoices created with a sk_test_ key. Test card tokens like pm_card_visa make payments succeed without any real card.
A Stripe Test Clock. A test-mode customer can be attached to a clock that starts frozen at a chosen time. You then advance the clock, and Stripe runs every billing event that would have happened in between: the trial ends, the first invoice is created, payment is attempted.
A Neon branch of the production database. Branching gives a copy of the schema (and data) that can be written to freely and then deleted. The production database is never connected.

I wrote one script with a subcommand for each step, so every step could be run, inspected, and repeated on its own: setup, reward, advance, assert, reset-pending, cleanup. Each subcommand prints a single JSON line with IDs, statuses, and counts, and never prints connection strings or keys.

Guards first

A test script that creates customers and writes rows is one environment variable away from doing that in production. So before any subcommand does anything, it runs the same guard function:

// Never write to a DATABASE_URL whose hostname doesn't start with this
// (prevents accidental writes to production or other branches)
const S8_BRANCH_ENDPOINT = "ep-xxxxxxxx"; // the test branch's endpoint, redacted here

async function assertGuards(): Promise<{ stripe: Stripe; priceId: string }> {
  const secretKey = process.env.STRIPE_SECRET_KEY;
  if (!secretKey || !secretKey.startsWith("sk_test_")) {
    printAndExit({ ok: false, reason: "STRIPE_SECRET_KEY_NOT_TEST_MODE" }, 1);
  }

  const connectionString = process.env.DATABASE_URL;
  if (!connectionString) {
    printAndExit({ ok: false, reason: "DATABASE_URL_NOT_SET" }, 1);
  }
  let hostname: string;
  try {
    hostname = new URL(connectionString).hostname;
  } catch {
    printAndExit({ ok: false, reason: "DATABASE_URL_UNPARSEABLE" }, 1);
  }
  if (!hostname.startsWith(S8_BRANCH_ENDPOINT)) {
    printAndExit({ ok: false, reason: "DATABASE_URL_NOT_S8_BRANCH" }, 1);
  }

  const priceId = process.env.STRIPE_PRICE_PRO_ANNUAL;
  if (!priceId) {
    printAndExit({ ok: false, reason: "STRIPE_PRICE_PRO_ANNUAL_NOT_SET" }, 1);
  }

  const stripe = getStripe();
  const price = await stripe.prices.retrieve(priceId);
  if (price.livemode === true) {
    printAndExit({ ok: false, reason: "PRICE_IS_LIVEMODE" }, 1);
  }

  return { stripe, priceId };
}

There are three independent checks: the key prefix, the database hostname, and the livemode flag on the price Stripe actually returns. The database check is an allowlist of one specific branch, not a denylist of production. A denylist would quietly allow any other database I happened to point it at.

The connection string for the branch went into a separate, git-ignored env file that the script loads on top of the normal one. I never needed to see its value, and the script never printed it.

Creating the world

setup builds everything a real referral would have produced:

const clock = await stripe.testHelpers.testClocks.create({
  frozen_time: Math.floor(Date.now() / 1000),
  name: "s8-referral-e2e",
});

const customerB = await stripe.customers.create({
  email: "s8-referred@example.test",
  name: "S8 Referred",
  test_clock: clock.id,
  payment_method: "pm_card_visa",
  invoice_settings: { default_payment_method: "pm_card_visa" },
  metadata: { s8: "1" },
});

const sub = await stripe.subscriptions.create({
  customer: customerB.id,
  items: [{ price: priceId }],
  trial_period_days: 14,
  discounts: [{ coupon: REFERRAL_COUPON_ID }],
  expand: ["latest_invoice"],
});

On the database side, inside one transaction, it inserts two users (referrer A and referred B), a trialing subscription row for B, and a pending referral row linking them. The .example.test email addresses are reserved for testing and can't belong to anyone.

One detail was deliberate: user A gets no subscription row at all. On the free plan in my app, a user has no Stripe customer and no subscription row until they first check out. The reward code has to create a Stripe customer for the referrer on demand before it can credit them. That lazy path is exactly the kind of branch that never runs in development, so the test environment was built to force it.

Test 1: the negative test, during the trial

Before advancing the clock, I ran the reward function against the subscription while it was still in its trial. The expected outcome was: nothing happens, and the referral stays pending.

It did stay pending. The interesting part was why. It would be easy to assume that during a trial the latest invoice is missing, or unpaid. It was neither. The subscription already had a latest invoice, its status was paid, and its amount_paid was 0.

When a subscription starts with a trial, Stripe creates a zero-amount invoice for the trial period and marks it paid. If my guard had been latestInvoice.status === "paid" on its own, which reads like a perfectly reasonable definition of "the customer paid", the reward would have fired the moment the trial started, with no money involved at all.

The amount_paid > 0 half of the condition was the only thing standing between a correct reward and a free one. It looks like a minor extra check when you read the code. On the most ordinary path in the system, the trial, it is the check doing all the work.

Test 2: advancing the clock past the end of the trial

advance moves the Test Clock to a point a few hours after the trial ends, then waits for Stripe to finish processing:

await stripe.testHelpers.testClocks.advance(state.testClockId, {
  frozen_time: state.trialEnd + 3 * 3600,
});

const startedAt = Date.now();
let ready = false;
while (Date.now() - startedAt < ADVANCE_TIMEOUT_MS) {
  const clock = await stripe.testHelpers.testClocks.retrieve(state.testClockId);
  if (clock.status === "ready") {
    ready = true;
    break;
  }
  await sleep(ADVANCE_POLL_MS);
}

Advancing a clock is asynchronous. The API call returns right away, and the clock reports a non-ready status while Stripe works through the simulated time. Reading the subscription before the clock is ready gives you a state that is halfway through the transition. That's another way to fool yourself, so the script polls until ready and fails loudly on timeout.

After the advance, the subscription was active, the latest invoice was paid, the amount paid was exactly the discounted annual price, and the billing_reason was subscription_cycle. That's the state my production webhook would see.

The surprise: `invoice.subscription` was null

The webhook handler for invoice.paid has to find out which subscription the invoice belongs to. Older Stripe API versions put that on invoice.subscription. Newer versions, starting with the release Stripe calls Basil, moved it to invoice.parent.subscription_details.subscription. My SDK version and my webhook endpoint's API version were not the same, so I had written the extractor to check both:

// Invoice -> subscription ID. Basil and later: invoice.parent.subscription_details.subscription.
// Older versions: invoice.subscription. Check both, because the SDK's pinned version and the
// webhook endpoint's version differ.
function extractSubscriptionId(invoice: Stripe.Invoice): string | null {
  const inv = invoice as unknown as {
    subscription?: string | { id: string } | null;
    parent?: {
      subscription_details?: { subscription?: string | { id: string } | null } | null;
    } | null;
  };
  const raw = inv.parent?.subscription_details?.subscription ?? inv.subscription ?? null;
  if (!raw) return null;
  return typeof raw === "string" ? raw : raw.id;
}

When I wrote that fallback, I wasn't sure it was needed. It might just have been defensive noise. So the advance step logged both paths separately on the real invoice. The new path had the subscription ID. The old path was null.

Without the new path, handleInvoicePaid would have returned early on every invoice. No error, no log line, just a referral sitting in pending forever. Because this is an annual plan, "forever" is not an exaggeration: there is no second chance until the renewal a year later. That's the kind of bug a Test Clock is good at finding, because it hands you a real Stripe object of the current shape rather than the shape you remember.

Test 3: granting the reward, and proving it happened exactly once

With the subscription in its paid state, I ran the reward path through my manual sweep script, first as a dry run and then for real. Then assert checked the outcome from three independent angles:

the referrals row for this subscription was rewarded, and there was exactly one row;
the referrer now had a subscription row and a Stripe customer, created lazily by the reward code;
the referrer's Stripe balance held exactly the expected credit, and exactly one balance transaction contained the marker string (referral <id>).

"Exactly one" was the whole point. A reward system that double-pays is worse than one that pays late, because a late payment can be fixed with a sweep and a double payment means asking someone to give money back.

Re-running everything

First, I simply ran the sweep again, and then called the reward function directly. Nothing changed. The referral was no longer pending, so the function returned immediately.

Second, I tested the failure mode that worried me more. The reward code calls Stripe with an idempotency key, so a retried request inside Stripe's retention window returns the original result instead of creating a second credit. But Stripe's docs say idempotency keys can be removed after they are at least 24 hours old. If the Stripe call succeeded but my process lost the response, the code rolls the referral back to pending. If the retry then happens a day later, the idempotency key no longer protects anything.

To simulate that, reset-pending forces the row back into the "we don't know what happened" state:

const updated = await db
  .update(referrals)
  .set({ status: "pending", rewardedAt: null, stripeBalanceTransactionId: null })
  .where(eq(referrals.id, state.referralId))
  .returning({ id: referrals.id });

Then I ran the sweep again. This time the reward code reached its second line of defense. It listed the referrer's balance transactions on Stripe, found the existing credit by its marker, recorded that transaction's ID, and confirmed rewarded without creating a new credit. The balance transaction count stayed at one.

That second line of defense came out of a review, and up to this point it had only ever been read, never executed. Now it had run against a real Stripe API response.

Cleanup is part of the test

cleanup deletes the Test Clock, then deletes the test referral, subscription, and user rows from the branch and prints how many rows of each it removed. After that I deleted the database branch itself and the temporary env file that held its connection string.

I treat cleanup as a test step rather than an afterthought. A leftover database branch with a real-looking schema, and an env file pointing at it, is something future-me could connect to by mistake months later. A test that leaves debris behind has side effects, and side effects are exactly what I was trying to keep away from production in the first place.

What this test did not prove

This setup proved the reward logic: the eligibility conditions, the lazy customer creation, the compare-and-swap, the double-payment defenses, and the invoice shape on the current API version. It did not prove:

that the production webhook endpoint actually receives invoice.paid. My script called the reward function directly instead of going through the webhook, so it can't see the endpoint's configuration at all. This one wasn't hypothetical: when the switch to invoice.paid was made, a review added "confirm the endpoint is subscribed to invoice.paid" as a separate pre-launch check. When I looked at the production endpoint, it wasn't subscribed, and I had to add the event by hand;
that the live Checkout page applies the referral discount correctly (I checked that separately, by creating a real Checkout Session with the same parameters and looking at the page, without completing it);
that a real second person can go through the whole flow in the UI.

I wrote those gaps down next to the test results, in the same document. A test report that lists only what passed invites the next reader, usually me, to assume it covered everything.

What I'd generalize from this

Any condition based on "paid" needs a test that runs while a trial is still in progress. Zero-amount invoices are real invoices with a real paid status. If your logic means "money moved", say so in the code with an amount check, and make sure a test exercises the trial state, not just the after-trial state.

Time-dependent billing logic deserves a clock you control. If the event you care about happens once per customer, at the end of a trial or a renewal, don't wait for it and don't fake it by calling your handler with a hand-written payload. A Test Clock gives you real Stripe objects in their real current shape, which is exactly what exposed the invoice.subscription: null problem.

Fence your test harness with allowlists, not hope. Key prefix, a specific database hostname, and the livemode flag of an object fetched from the API. Three cheap checks, each one independent of the others, all of them run before the first write.

Test your second line of defense by deliberately breaking the first. Idempotency keys expire, responses get lost, rows get rolled back. If your defense against that is "look for an existing record before writing", the only way to know it works is to put the system into the ambiguous state on purpose and run it.

Build the code path you can't reach in development. My lazy customer creation only runs for a referrer who has never checked out. In normal development I always have a subscription, so it would never have run. The test data was shaped specifically to force that branch.

Write down what the test didn't cover, and give each gap its own check. The most important production misconfiguration in this feature, a webhook endpoint not subscribed to the event that triggers the reward, sat entirely outside what my test could see. A green test run would never have revealed it. It was caught only because it had its own line on the pre-launch checklist, separate from the test.

Most of the harness is guards and output formatting. The Test Clock calls themselves are a small fraction of it. That ratio seems about right to me: moving time is easy, and making sure you only moved time where you meant to is the part that takes the effort.

I moved my webhook work into waitUntil() to fix a timeout. The failures didn't go away, they just stopped being visible.

phi_blankslate — Wed, 14 Oct 2026 01:11:34 +0000

My GitHub App takes a pull request, asks an LLM to review it, and posts the comments back. The first time I ran it end to end against a real repository, the review showed up on the PR exactly as intended. Then I opened GitHub's webhook delivery log and found the delivery marked as failed.

Both things were true at once: the feature worked, and the delivery had failed. That contradiction is the interesting part, and the fix I reached for first created a worse problem than the one it solved. This post walks through both, with the code, because the second problem is the kind that doesn't announce itself.

The stack is Next.js (App Router) on Vercel, Drizzle and Postgres, but nothing below depends on those. If you run slow work behind a webhook on a serverless platform, you will meet the same shape.

The symptom: a successful review and a failed delivery

The handler was the obvious one. Verify the signature, parse the payload, do the work, return 200:

case "pull_request":
  await handlePullRequestEvent(payload);
  break;

handlePullRequestEvent fetches the changed files, calls the model, and posts a review. The model call alone regularly takes longer than ten seconds.

GitHub's webhook best-practices page says a delivery should respond within 10 seconds, and in my notes from this incident GitHub does not automatically redeliver a failed one. So the sequence was:

GitHub sends the pull_request event.
My handler is still waiting on the LLM when GitHub gives up at about ten seconds and records the delivery as failed.
My function keeps running anyway, because the serverless runtime doesn't know the caller hung up. It finishes, posts the review, and writes the result to the database.

So the user-visible outcome was fine, and the only evidence of a problem was a red mark in a log that nobody looks at until something else goes wrong. That is a bad state to leave a system in even when the feature technically works: the next time a delivery fails for a real reason, it will look exactly like this one.

The cause is simple. I was doing work whose duration I don't control inside the request/response cycle of a caller that gives me a fixed budget.

The obvious fix

Acknowledge immediately, do the work afterwards. On Vercel the primitive for "keep running after the response is sent" is waitUntil from @vercel/functions:

import { waitUntil } from "@vercel/functions";

// ...
case "pull_request":
  // Generating the review often takes longer than 10 seconds, which is longer
  // than GitHub's delivery timeout (about 10s, one attempt). ACK immediately
  // and keep working in the background.
  waitUntil(
    handlePullRequestEvent(payload).catch((error) => {
      console.error("PR review background processing failed:", { deliveryId }, error);
    })
  );
  break;

Everything before this point is cheap and synchronous: verify the HMAC signature, parse the JSON. Then the response goes out, and the review is generated in the background.

It type-checked and the change was small, so I was pleased with it. Before shipping, I ran a security review pass over the diff, and it pointed at the exact line I was pleased with. After I addressed that finding (below), I deployed, re-ran the end-to-end test, and the delivery log showed green.

Why this is worse than it looks

Before the change, a failure had a natural place to show up. If the handler threw, GitHub got a 500, and the delivery was red. The work and the acknowledgement were the same event, so a problem with one was a problem with the other.

After the change, the acknowledgement says only "I received your payload and the signature was valid." It says nothing about whether the work will finish. The failure modes of the background job are now invisible to the caller by construction:

The function hits its platform time limit and is killed.
A deploy replaces the running instance.
The process runs out of memory.
An upstream call hangs and never returns.

None of those run your .catch. A killed process doesn't get to log anything or update any row. The delivery log stays green, because the delivery succeeded. What's left is whatever state you wrote before you started the work.

In my case that state was a row in the reviews table. I create it with status = 'pending' before returning, and the background job moves it to completed or failed at the end. If the job dies in the middle, the row stays pending for good.

That sounds like a cosmetic problem, a spinner that never stops. It isn't, for a reason specific to how I count usage. A pending row counts toward the user's monthly quota, deliberately:

// Rows that count toward the quota:
//   pending (in progress) counts too. If only completed rows were counted,
//   concurrent webhooks that arrive before an earlier review finishes would all
//   be judged to be within the limit, making the free tier effectively unlimited.
//   failed rows are excluded in principle, but a row that failed after the
//   Claude API was called (tokens_used > 0) still counts...
function countsTowardQuota() {
  return or(ne(reviews.status, "failed"), gt(reviews.tokensUsed, 0));
}

I count pending rows because otherwise a burst of simultaneous events all look like they fit under the limit. That's correct, and it means a row that is stuck as pending is not just an ugly record. It permanently consumes someone's allowance. A killed background job turns into a user being told they've used reviews they never received.

So the second lesson, after "don't hold the caller hostage," is that moving work out of the request cycle doesn't remove the failure modes. It moves them to a place where nothing is watching.

Layer 1: make the work end before the platform ends it

The first thing to do about a job that can be killed silently is to make sure it gets a chance to fail loudly instead. That means giving it an explicit time budget that is shorter than the platform's.

Without an explicit limit, the route depends on the platform default, and my comment in the code says what I found out about that:

// Give the background processing (PR file fetch + Claude API review generation +
// GitHub post) enough time. Leaving this unset depends implicitly on the Vercel
// default and fails silently when interrupted (reviews stay "pending" and become
// undetectable).
export const maxDuration = 120;

And the model client gets a timeout that is strictly below it:

// Keep this shorter than maxDuration (the webhook background limit) so a hung
// API call surfaces as an error before the platform timeout (silent-fail guard).
return new Anthropic({ apiKey, timeout: 90_000 });

The ordering matters more than the specific numbers. If the client timeout is longer than the function limit, the platform wins the race, kills the process, and nothing runs catch. If the client timeout is shorter, the SDK throws, my catch block runs, and the row is marked failed with an error message. A timeout I control turns into a status I can query. A timeout the platform controls turns into silence.

There is one caveat I want to be upfront about, because it's the kind of thing I'd want to know if I were copying this. A 90-second timeout on the client is a per-request limit. As far as I know, the Anthropic SDK retries failed requests by default, a couple of times. I have not set that option in this codebase and I have not tested what happens when a request times out and is retried. If retries do apply to timeouts, then the total time spent could be several multiples of 90 seconds, which is well beyond the function limit and would put the platform back in charge. I'm flagging that as something I still need to check rather than as a finding. The general point holds either way: when you stack a timeout under a deadline, budget the total across all attempts, not the single attempt.

Layer 2: assume the first layer will sometimes fail anyway

Timeouts catch the hung call. They don't catch a deploy that replaces the instance mid-flight, or an out-of-memory kill, because in those cases no code of mine gets to run at all.

So the cleanup can't live in the dying process. It has to be done by something else, later. I didn't want to add a scheduler and a new moving part for a problem this small, so the reaper runs lazily: the next time the same user's webhook arrives, it sweeps their stale rows before counting anything.

// Turn stuck pending rows (function cut off, OOM, replaced by a deploy, never
// reaching completed/failed) into failed. Rows that died before the Claude call
// (tokens_used=0) drop out of the quota; rows that died after it (the webhook
// writes a sentinel tokens_used>=1) stay in the quota even once failed.
// The threshold is 10 minutes, comfortably longer than maxDuration (120s).
const STALE_PENDING_MINUTES = 10;
async function sweepStalePending(
  tx: Pick<typeof db, "update">,
  userId: string
): Promise<void> {
  const cutoff = new Date(Date.now() - STALE_PENDING_MINUTES * 60_000);
  await tx
    .update(reviews)
    .set({
      status: "failed",
      errorMessage: `stale pending swept (older than ${STALE_PENDING_MINUTES}m)`,
      completedAt: new Date(),
    })
    .where(
      and(eq(reviews.userId, userId), eq(reviews.status, "pending"), lt(reviews.createdAt, cutoff))
    );
}

Three decisions in there are worth spelling out.

The threshold is derived from the limit. Ten minutes is not a round number I liked. It has to be comfortably larger than maxDuration, otherwise the sweeper could mark a healthy, still-running job as failed while it is still working. The sweep is only safe because "older than ten minutes" means "the platform has already killed it", and that holds only as long as the threshold stays above the function limit.

It runs inside the same transaction and the same lock as the check it protects. The sweep is called from the function that reserves a review slot, after taking a per-user advisory lock:

await tx.execute(sql`select pg_advisory_xact_lock(${LOCK_NS_USER}, hashtext(${userId}))`);

// Reap stale pending rows under the lock, then count (serialized against
// concurrent runs for the same user)
await sweepStalePending(tx, userId);

The point of doing the sweep under the lock is that the count which decides "is this user over the limit" is computed on cleaned-up data, and no other request for that user can interleave between the cleanup and the count.

Whether a swept row still costs the user depends on how far it got. This is the part I like least to have to think about, and the part I'm happiest I did think about. A job that died before it called the model has cost nothing, so its row should fall out of the quota. A job that died after the model call started has cost real money, and if it were free it would be an exploit: anyone who could make the post-generation step fail on purpose would get unlimited generations. So the webhook writes a sentinel immediately before calling the model:

// Write a sentinel (tokens_used=1) just before the Claude call. If the call
// times out or the function is cut off before completing, the row still counts
// toward the quota as "generation started, cost may have been incurred". With
// tokens_used=0, a failed/swept row would free the quota and let someone abort
// generations to burn Claude cost for free.
await db.update(reviews).set({ tokensUsed: 1 }).where(eq(reviews.id, reviewRecord.id));
tokensUsed = 1;

The sweeper then doesn't need to know anything about what the job was doing. It just flips pending to failed, and the previously written tokens_used decides whether that row still counts. The state machine carries the accounting.

The reaper created a new problem: a retry budget it can exhaust

Once stuck rows get turned into failed, something else has to decide whether that commit may be reviewed again. A webhook can be redelivered, and a user can close and reopen a PR, so the slot reservation does two checks before it allows another run for the same (repository, PR, commit):

// 1-a: already delivered (completed / pending) -> reject immediately
const delivered = await tx
  .select({ count: count() })
  .from(reviews)
  .where(and(sameCommit, ne(reviews.status, "failed")));
if ((delivered[0]?.count ?? 0) > 0) {
  return { allowed: false, plan, used: 0, limit, record: null, reason: "duplicate" as const };
}

Note that pending counts as "delivered" here. That is what stops a redelivered webhook from running the same review twice while the first is still going. It also means that a stuck pending row blocks the commit entirely until the sweep clears it, which is one more reason the sweep has to exist.

The second check caps total attempts, because without a cap a close/reopen loop could trigger paid calls forever:

const attemptsSince = new Date(Date.now() - ATTEMPT_WINDOW_MS);
const attempts = await tx
  .select({ count: count() })
  .from(reviews)
  .where(and(sameCommit, notQuotaNotice(), gte(reviews.createdAt, attemptsSince)));
if ((attempts[0]?.count ?? 0) >= MAX_ATTEMPTS_PER_COMMIT) {
  return { allowed: false, plan, used: 0, limit, record: null, reason: "attempt-cap" as const };
}

The gte(reviews.createdAt, attemptsSince) window is the interesting line, and it exists because of the reaper. A review caught by the sweep is a failed row, and failed rows count as attempts. My own comment in the code explains the consequence of leaving the window out: the "zero-cost abort" rows the sweeper creates would pile up, and after a few infrastructure accidents of my own making, such as deploys replacing a running instance, that commit could never be reviewed again. The user would have done nothing wrong, and my cleanup logic would have permanently blocked them.

I only found that by thinking about the reaper as a producer of data, not just a consumer. Every row it writes feeds the other rules in the system. If you add a janitor, trace what its output does to every check that reads the same table.

What this still doesn't cover

I'd rather list the gaps than have you find them.

The sweep is lazy. It runs when that user's next webhook arrives. If they never send another, their stuck row stays pending. The place that displays the monthly count is read-only, so a stuck row keeps showing as used until the next event cleans it up. I decided that was acceptable for the volumes involved, and I wrote the decision down in a comment, but it is a real gap, and a scheduled sweep would close it.
A swept review is simply gone. The user's PR doesn't get a review, and nothing retries it. A new push produces a new event with a new commit, which is a fresh attempt. I left it that way because automatic retries on a billable operation are their own can of worms.
The time budget is reasoning, not measurement. The 120, the 90 and the 10 come from how I reason about the pieces interacting. I haven't load-tested a run where several of those limits are approached together.

The general lesson

If you only take one thing away: acknowledging a webhook early doesn't make the work reliable, it makes the work unobserved. Before the change, a slow job turned the delivery red, which was annoying but honest. After it, the delivery is always green, and the only truth left is the state you wrote to your own database. So that state has to be designed:

Write a durable record before you acknowledge. The row exists before the response goes out, so there is something to inspect if the job vanishes.
Give the work a deadline shorter than the platform's, and order the timeouts. Make your own timeout fire first, so failure arrives as an exception you can handle rather than a kill you can't.
Put the cleanup outside the process that might die. A reaper that runs from somewhere else, later, is the only thing that works when catch doesn't.
Derive the reaper's threshold from the limit, and decide in advance what a reaped row costs. If "failed" means "free," someone will find out how to fail on purpose.

The webhook stayed fast, the delivery log stayed green, and the failure mode that used to be loud became a row with an error message and a timestamp. That is a better place for it to live. But I only got there after a review pass pointed at the one line I thought I'd finished with, which is a good reminder that "the red thing turned green" is not the same as "the problem is solved."

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

I almost killed a project because my test harness was measuring its own bug

The question I was trying to answer

The free run

Bug 1: axe-core's node.html is not the element

Why this bug points toward "pass"

The detection trap

The fix: re-fetch the real element

Bug 2: I was counting other people's bugs as my customer's

Why measurement bugs lean toward "pass"

A quick checklist for "is my harness lying?"

What I do now

The uncomfortable bit

How We Secure DevReview (October 2026)

What outside scanners say (measured 2026-10-24)

Your code

The doors into our servers

How we run things behind the scenes

What is not done yet

Tips for your side

Found something?

Stripe marks a subscription "active" before anyone has paid. Here's how I tested a reward that has to wait for real money.

The bug that shaped the design: "active" is not "paid"

Why the obvious test plans didn't work

The setup: Stripe test mode, a Test Clock, and a throwaway database branch

Guards first

Creating the world

Test 1: the negative test, during the trial

Test 2: advancing the clock past the end of the trial

The surprise: invoice.subscription was null

Test 3: granting the reward, and proving it happened exactly once

Re-running everything

Cleanup is part of the test

What this test did not prove

What I'd generalize from this

I moved my webhook work into waitUntil() to fix a timeout. The failures didn't go away, they just stopped being visible.

The symptom: a successful review and a failed delivery

The obvious fix

Why this is worse than it looks

Layer 1: make the work end before the platform ends it

Layer 2: assume the first layer will sometimes fail anyway

The reaper created a new problem: a retry budget it can exhaust

What this still doesn't cover

The general lesson

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

Bug 1: axe-core's `node.html` is not the element

The surprise: `invoice.subscription` was null

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

Why `cancel_at_period_end` stayed false

If you use `cancel_at_period_end` today