I'm going to be straight about this; this code review sent me into a soul search.
The code review left BoomerClip with a five-phase fix plan. The exciting phases come later: making deletion a server guarantee, one video player for every browser. Phase 1 was the unglamorous one, and I called it clear the ground.
The reasoning is simple. You can't fix a security issue with confidence in a codebase that doesn't compile, has no tests, and is half made of code nobody runs. So before fixing anything, I made it possible to know whether a fix worked.
Deleting what never ran
The review had found an entire "enterprise" layer: an alternative server entry point, plus rate limiting, monitoring, alerting and compliance modules. None of it had ever run.
Deleting code feels dangerous even when you're sure, so I checked first. An import scan confirmed nothing in the running server touched any of those six files. Then they went, and the server's type-check went from 53 errors to zero.
Dependencies were next. 25 packages came out: 22 from the server and 3 from the client. Nine of them weren't imported by anything at all, and the rest only served the deleted layer. I re-checked each one against the codebase before removing it, because "the review says it's unused" is a claim, not a fact.
Some of the dead code was archaeology. The utilities and server config from BoomerClip's original live WebRTC version were still sitting there, more than a year after the pivot to recordings.
And one find was genuinely uncomfortable: two debug pages from 2025 were shipping in every build. They were test pages that started screen capture with no consent step. For an app whose whole identity is "your recording doesn't stick around", having pages like that in the public folder is exactly the wrong look. They're gone.
A build that builds
npm start pointed at a file the build didn't produce. That was easy to fix, and it uncovered something more interesting.
Every storage path in the server was worked out relative to the file that used it. That's fine in development, where the code runs from src/. But once compiled, the code runs from dist/, so the built server would have looked for recordings in an empty folder inside dist, while the development server used the real one. The two versions of the app would have quietly disagreed about where the videos were.
Storage paths now come from config, resolved from the project root, so the development server and the built server see the same files.
One more production surprise: any unknown /api/... address returned the React app's HTML page with a 200 OK. So a typo in an API call looked like success, and returned a web page instead of data. Unknown API paths now return a proper JSON 404.
The fix that reopened a hole
This is the lesson of the phase.
There was an old route for downloading recordings. The review had already flagged that it never checked who owned the recording it served. But it had been harmless by accident: its file path was wrong, so it could never find a file.
Then I fixed the storage paths.
Suddenly the route could find every file, and it still didn't check ownership. Fixing one bug had turned a broken feature into a working hole. I caught it while writing up the phase notes, not from a test. The route is now unmounted until it can check ownership properly, committed separately, and confirmed to return a 404.
The line I wrote down afterwards: a path fix is a behaviour change for every caller, including the broken ones.
Tests that test the real thing
BoomerClip had no automated tests. The first ones went where the product's promise lives: the deletion lifecycle.
The tempting shortcut was to mock Redis. But the behaviour I needed to test (keys expiring, keys being deleted, timers running out) is exactly what a mock would be pretending to do. A deletion test that passes against a fake proves very little about deletion.
So the tests run against:
- Real Redis, on its own numbered database, with a guard that refuses to run against the default database, where my real local recordings live.
- A temporary storage folder, so tests never touch real files.
- A real Socket.io server and client, exercising the same leave and reconnect flow the viewing page uses. The 15-second reconnect grace period is a parameter, so tests don't have to sit and wait for it.
That's 10 tests passing, and 4 marked as to do. Those four are placeholders for the guarantees phase 2 has to deliver, so the next phase starts with its finish line written down.
Breaking my own code on purpose
A test suite that has never failed hasn't proven anything. So once the tests passed, I deliberately broke the code four ways: skipping the thumbnail deletion, skipping the ephemeral check, breaking the grace timer, and breaking the reconnect cancellation. Every one failed a test.
One experiment didn't fail anything: removing the line that cancels the grace timer when a viewer comes back. I dug into why, and it wasn't a gap. When the timer fires, it re-checks whether the viewer is still connected, so the recording survives either way. Two different paths reach the same correct behaviour. That's a relief to confirm rather than assume.
Small things that took longer than they should
- Watching a recording deletes it, which makes manual testing wonderfully self-defeating. A new
clone-recordingscript copies a local recording under a new ID, so I can test the viewer without using up my test videos. - The review had counted 702 formatting problems. Most weren't real: Git on Windows was checking files out with Windows line endings, and the formatter wanted Unix ones, so every line of those files was flagged. The repo now pins its own line endings for every machine. After a formatting pass and a few hand fixes, lint reports zero errors.
- Reinstalling dependencies failed on Windows because the running dev servers held native library files open. Rather than stop everything, I verified the whole setup from a fresh clone: install, build, type-check, test, lint.
- The first test clean-up ran in the wrong order. It tried to delete test data after closing the Redis connection, so test keys lingered.
- Remember the duration display from the early days? While writing the clone script, I found recordings still store their duration as the text
"N/A". That's logged for phase 2.
Where it stands
Phase 1 is done. BoomerClip builds, starts in production mode, passes its tests, lints clean, and carries a lot less code than it did.
Phase 2 is underway now: making deletion a guarantee the server enforces, and fixing the security findings from the review. Each fix gets a test, and those four to-do tests are the first ones due.
~ Jer