Noctem Labs

Approve all approved everything except the backlog it existed to clear

Batch approval resolved "all" through getSlots({from: new Date()}), which renders as slot_at >= now. Every past-due filled slot was invisible to the one command that exists to approve it. It approved

Repositoryclaude-editor
Detected2026-07-23
Fixed2026-07-23
Commitsaf12a04

Batch approval said ok. That's the whole horror story, right there. Not an error, not a crash, just a calm little "ok" while the backlog sat untouched behind it, growing.

Symptom

Someone ran batch approval on "all" and it worked, technically. It returned ok. It did not throw. It did not complain. What it actually did was resolve "all" through getSlots({from: new Date()}), which renders as slot_at >= now, and then approve everything that matched. Every slot in the future got approved, clean as anything. Every past-due filled slot, the actual backlog, the thing "approve all" exists to clear, was invisible to it. The command ran, told the truth about running, and skipped the one job it had.

Nobody noticed straight away, because "ok" is a satisfying thing to see on a screen. You run a batch job, you get ok back, you move on with your day. The failure wasn't loud. It was quiet in exactly the way that lets a backlog sit for a while before anyone asks why it's still there.

What we thought it was

First guess: a permissions issue. Somebody somewhere assumed the slots weren't being approved because the calling user lacked rights on older records, maybe some ownership check failing silently. That felt believable for about ten minutes, because permission bugs love to hide behind vague success responses, and we'd seen that pattern before in other parts of the system.

Second guess, once permissions checked out clean: a stale cache. Maybe the approval service was reading from a snapshot that hadn't caught up, so past-due slots looked already-handled even though they weren't. This one was more seductive, honestly, because cache bugs are the perfect villain. They're invisible, they're intermittent-feeling even when they're not, and they let you avoid looking at your own query logic for another half hour.

Both guesses were wrong. Both guesses were also, in fairness, reasonable places to look first, because the actual bug doesn't announce itself in any of the places engineers instinctively check. Nobody opens the "resolve all" logic first. You check auth, you check caching, you check the obvious suspects. The query itself just sits there, looking fine.

Root cause

It wasn't permissions. It wasn't a cache. It was a filter.

In claude-editor, src/approve.ts:5, the resolution of "all" calls getSlots({from: new Date()}). That call compiles to slot_at >= now. Read that again slowly, because it's doing exactly what it says and that's the problem. "All" was never actually all. It was "all future slots," silently, by construction, because someone wrote a default that made sense for a different use case and then "all" inherited it without anyone deciding that on purpose.

Past-due filled slots, the ones actually waiting on approval, sat entirely outside that window. The query didn't fail to find them. It never looked. From the database's point of view there was nothing wrong, because nothing wrong was asked of it. The bug wasn't in what the code did with the data. It was in what data the code decided to ask for in the first place.

Fixed against commit af12a04, dated 2026-07-23, same day it was detected. The gap between finding it and fixing it was basically the time it took to read the one line.

The fix

The change removed the implicit from: new Date() constraint from the "all" resolution path, so that approving "all" actually means all pending slots regardless of when they're timestamped, not all pending slots from now onward. Past and future are both in scope now, which, again, is what "all" is supposed to mean. It's not a clever fix. It's barely a fix at all in terms of line count. That's sort of the point.

Why it happened

This is the part worth sitting with, because the bug itself is small and the reason it survived is not really about that line of code at all.

getSlots({from: new Date()}) is a completely correct, completely reasonable default somewhere else in the system. Probably in a dashboard view, or a "what's coming up" list, somewhere the intent genuinely is future-facing. It got reused for "all," and reuse is exactly how defaults escape their original context. The function didn't lie. It did what it was told. The problem is that "resolve all" told it the wrong thing, and nobody had to actively decide that, it just happened by inheritance.

Code review didn't catch it because the line reads fine in isolation. from: new Date() looks like a sensible, almost cautious choice, not a scope-limiting one. You'd have to already know that "all" needs to include the past to flag it, and if you already knew that, you wouldn't have written the bug in the first place. Review works well against code that looks wrong. It's much weaker against code that looks conservative and reasonable while quietly doing less than it claims to.

And it survived testing, presumably, because tests for "approve all" were written against future or current slots, because that's the natural thing to construct a test around. Nobody wrote a test with a deliberately stale, past-due, still-pending record, because nobody was imagining that case as the dangerous one. The dangerous case was the boring one. It always is.

How to avoid it

If you've got a function whose name implies totality (all, everything, entire backlog, whatever the word is in your system), audit every filter hiding inside it, explicitly, on purpose, not by reading the code once and nodding. Ask what "all" excludes, not what it includes. Exclusion is where these bugs live, because inclusion is what gets tested and exclusion is what gets assumed.

Watch for defaults that migrate. A parameter that's correct for its original call site can become wrong the moment it's reused somewhere with a different meaning of the word "all," and nothing about the function signature will warn you when that happens.

And build at least one test around data that's inconveniently old. Not the happy case, not the recent case, the case that's been sitting there a while, unapproved, unnoticed, exactly the kind of record a backlog is made of. If your test suite only ever exercises fresh data, you'll never see a bug that only bites stale data, because you never gave it the chance to.

The fix here took one line. Finding it took noticing that "ok" doesn't mean "done," it just means "didn't fail." Those are not the same claim, and a lot of quiet damage happens in the gap between them.

Every commit, file reference and date above is checked against this incident's evidence record before publication. Evidence: editor-approve-skipped-past-due.