What an AI code reviewer actually caught


For the last few weeks a bot has reviewed every pull request I open on this site. It's Cursor Bugbot, and its whole job is to read a diff and try to find the bug I missed. I merge almost all of this work as a solo developer, so a second reader that never gets bored and never assumes I meant well is worth having around.

The honest question is whether it earns its keep. Automated review has a reputation for noise: a pile of "consider extracting this function" comments that bury the one thing that matters. So I went back through three PRs where Bugbot flagged something and asked a plainer question about each finding. Was it real? Would it actually have bitten me? And could I have caught it some cheaper way first?

The answer surprised me in a specific direction. The best catches were not style, and they were not logic errors a test would have found. They were failures that only exist against real infrastructure, the kind that pass every local check and then throw the first time they run in production.

The one that would have failed on the very first run

I added a daily encrypted backup of the production database. A cron hits an endpoint, the endpoint exports every collection to JSON, gzips it, encrypts it with AES-256-GCM, and uploads the result to Vercel Blob. I tested the export. I tested the encryption. I ran tsc. Everything was green.

Bugbot flagged one line in the upload call:

The backup upload passes cacheControlMaxAge: 0 to @vercel/blob put, but the SDK requires at least 60 seconds. That validation typically throws during upload, so authorized cron runs can fail after export and encryption and no new backup is stored.

I had set the cache age to zero because a backup should never be cached. Reasonable intent, wrong value. The Blob SDK rejects anything under its minimum, and it checks at upload, the last step. So the export would run, the encryption would run, then the whole thing would throw right before saving anything. A backup job that fails silently and stores nothing is worse than none, because you think you're covered.

Here's what makes this the catch I keep pointing to: nothing local could have found it. My types were fine. My logic was fine. The value was only invalid against the actual Vercel Blob API, which I wasn't calling in any test. The first time I'd have learned about it is the day I needed a backup and went looking for one that was never there. Bugbot rated it High. It was right.

The one that would have deleted the thing it just made

A little later I wrote a pre-migration snapshot script for Neon. Before a production migration runs, it takes a database branch as a backup, then prunes old snapshots so they don't pile up forever. How many to keep is configurable through NEON_PREMIGRATION_KEEP.

Bugbot's finding:

The NEON_PREMIGRATION_KEEP environment variable is coerced to a number without validation. If it's 0 or a non-numeric string, Number() yields 0 or NaNArray.prototype.slice treats these as 0, causing the pruning logic to delete all pre-migration-* branches, including the backup just created.

Sit with the timing on that one. The script exists for exactly one moment: the seconds before a schema migration touches production. If someone set the keep count to 0, or fat-fingered it into something non-numeric, slice(KEEP) would quietly become slice(0), and the prune step would delete every snapshot, including the one it had made ten lines earlier. The safety net removes itself, right before the tightrope walk.

This is the failure the script was written to prevent, hidden inside the script meant to prevent it. My fix was to validate the value to an integer of at least one and fall back to a sane default otherwise. I noted in the PR reply that this was "the scary one," and I meant it. A reviewer looking at the diff has to hold the runtime consequences of slice and the operational meaning of the moment in their head at the same time. Bugbot did.

The one hiding in a database cascade

The third PR added post series to the blog, the little "Part 2 of 4" banner you might have noticed. Series membership is a relationship in the database, and when a post is deleted I want its siblings to refresh so they stop advertising a post that no longer exists.

I wired that refresh into an afterDelete hook, which felt like the obvious place. Bugbot disagreed:

The revalidateSeriesSiblings call in afterDelete fails because ON DELETE CASCADE removes the post's series relationships before the hook runs, preventing it from finding any series to revalidate.

That is a precise reading of an ordering problem. The relationship row has ON DELETE CASCADE, so by the time afterDelete runs, the link between the post and its series is already gone. The hook goes looking for the series to refresh and finds nothing, because the thing it needed to find the series was cascaded out from under it. The siblings would keep showing the deleted post until I noticed and fixed the series by hand.

The fix was to move the work to a beforeDelete hook, while the relationship still exists, mark the sibling pages stale there, and let them regenerate after the delete commits. This is the sort of bug that hides comfortably in code that reads correctly line by line. Nothing is wrong with the hook. What's wrong is when it runs relative to a database behavior defined somewhere else entirely.

The one that leaked something I said wasn't there

Same backup PR, a different kind of catch. My whole framing for the backup was "no credential material": password hashes and reset tokens are hidden fields, so they never enter the export. Bugbot pointed out I'd drawn the line in the wrong place:

Payload's sessions array is admin-disabled but not hidden, unlike hash and reset tokens, so decrypted backups can contain active session IDs alongside the stated "no credential material" intent.

Active session IDs are a credential of sorts: hold one and you can act as that logged-in admin. They weren't hidden the way the password hash was, just tucked out of the admin UI, so they rode along into an export I'd described as safe. Lower drama than the others, but it's the finding that most directly contradicted a claim I'd made in my own PR description. A reviewer taking my summary at face value would have skipped right past it.

Where it over- and undershot

I don't want to oversell this. Bugbot flagged more than the headline catches, and calibration cut both ways.

Some findings were real but genuinely minor, and Bugbot mostly rated them that way. It flagged that my Blob and Neon list calls didn't follow pagination, so with more than a page of objects the prune could work on an incomplete set. Correct, and worth fixing for hygiene, but this is a personal site with a handful of backups and a handful of branches. I'll never hit a second page. Bugbot marked these Medium, which felt about right, maybe a touch high for my actual scale.

The rollback migration was a smaller version of the same story. My down() migration dropped a table with CASCADE and then tried to drop a constraint the cascade had already removed, so a rollback could error. Bugbot rated it Low. Also right. Rollbacks are rare and I'd have seen the error immediately, but the fix was one IF EXISTS guard, so why carry the sharp edge.

That's the honest limit: a bot rating severity is still guessing at blast radius. It can't know my "many branches" case will never happen, or that a rare rollback matters less than a daily backup. It gets the mechanism right and the impact roughly right. Whether roughly-right is worth acting on stays with me.

What it didn't flag is just as telling. Nothing about naming, structure, or the shape of my code, which is fine, because it isn't a taste engine. Nothing about design either, like whether a JSON export is even the right backup strategy. It reads a diff and asks "what breaks," not "is this the right thing to build."

What I actually take away

Three things.

Automated adversarial review is strongest exactly where my other nets are weakest. Types catch shape errors. Tests catch the logic I thought to test. Neither calls the real Vercel Blob API, knows the Neon SDK's minimum cache age, or reasons about when a Postgres cascade fires relative to a hook. Every catch I cared about lived in that gap: runtime behavior no local green checkmark can vouch for.

It complements those checks, it doesn't replace them. I still read my own diffs and still write tests. Bugbot is a fourth reviewer with a narrow specialty and no ego, and that specialty is the failures most expensive to learn about in production.

And the noise stayed manageable, because it labeled severity and mostly got it close. I could scan a High and a Low and spend attention accordingly. That's the line between a reviewer I keep and one I mute: right about mechanism, honest about confidence, and content to leave the final call to me.

Is this a reason to keep paying for Cursor?

That question is why I'm writing this at all. It's the one Alex Yumashev raises in Cancelling Cursor, where he drops the tool because it's stagnating as an editor while it chases enterprise agent tooling. His verdict is about Cursor the IDE. Mine is narrower and starts from a feature he doesn't touch. Bugbot is a Cursor feature, and I've been asking lately whether Cursor still earns its place next to Claude and plain VSCode, or whether I'm paying a second subscription for a shell around a model I can already reach other ways. This post is one entry in that longer reckoning.

The catches above are a point for Cursor. But the honest follow-up is whether they're a point for Cursor or just for automated PR review in general. If a reviewer not tied to Cursor reads the same diffs and flags the same cache-age minimum and the same self-deleting prune, then I value the category, not the vendor, and I can get it without paying for the editor. If it misses what Bugbot caught, that's a concrete reason to stay.

So that's the next experiment. I want to run this same kind of infrastructure PR past CodeRabbit and Greptile, two reviewers that plug into GitHub directly and don't ask me to live in a particular editor, and see what each one finds against the exact bugs I already know are there. Do they catch the Vercel Blob minimum that only fails against the real API? The slice(0) that deletes its own backup? The cascade that fires before the hook? That's a clean test, because I have the answer key. I'll write up the comparison as its own post.

The backup that would have saved nothing, the snapshot that would have deleted itself, the hook that fired a beat too late. None of them would have shown up until the day I needed them not to, and I'd rather meet that kind of thing in a PR comment than in an incident. The open question the rest of this series is chasing is which reviewer leaves the comment, and whether it needs to be Cursor at all.

Comments

No comments yet. Be the first to comment.

Leave a comment