Flow
Reviewing a two-into-one change
What would have caught the missing key check on my upload route.
1List the old checks
Every check each old route made, one line each.
2Read removed lines
A deleted check only shows up as red, so read those first.
3Question doc edits
A key dropped from the docs needs a reason, not a nod.
4Test with no key
A keyless upload should be refused before the file is read.
The PDF upload on my scheduling app stopped asking for a key. The change that did it was a good change in every other way.
The import used to take two requests. The browser sent the PDF and a token to an upload route, which checked the token, turned the PDF into text and sent it back. Then the browser sent that text to a parse route, which checked the token again and turned the text into shifts. Each route had its own copy of the check, typed at the top.
The change merged the two. The parsing code moved into a shared function, and the upload route now converts the PDF and parses it in one go. That's a fair move. The text no longer passes through the browser, where anyone could swap it.
But the upload route lost its token check. The browser stopped sending the token, and the docs dropped it from that route. The parse route kept its check. So the narrow route stayed locked and the bigger one opened up. My guess is the check fell out because it lived in the route, not in the parser that moved, and the route got rewritten. The commit doesn't say why.
The bigger route also hands back more. It reads positions and shifts from the PDF, matches the names against the private staff list, and adds each person's staff ID and lunch length from that list. It starts a converter process for every file, with no size cap or timeout in the change. A same-day follow-up added a debug switch, set from the request itself, that can log the converter command and return raw text, which is mostly names. And an older debug mode on the parse route already took text with no token.
Spotting this before it merges
When a change folds two requests into one, list every check each old route made and find where each one lives now. Read the removed lines first, because a deleted check shows up only as red. Here it hid inside a big restyle of a browser file. If the docs drop a required key, ask why instead of taking it as proof the change meant it.
Put the key check in one shared place that every file route goes through, not in each route body. The commit has no test that sends a file without a key, and that's the test I'd add first.