The guard that was never there

Greenhouse’s engine has a handful of functions that move a project through its lifecycle: promote an idea, log a touch, advance a stage, vault it, release it. Every one of them reads the item’s current state before deciding what to do. None of them checked whether that state was actually a sane place to be starting from.

That’s a strange thing to notice in code that’s been shipping for weeks. It worked, because the UI only ever offered these actions from the right place - a “Vault” button only shows up on an item you can vault. But the engine functions themselves didn’t enforce it. Call vault_item twice on the same item (a double click, a retry, a future caller that isn’t the current UI) and it would happily re-stamp the vaulting timestamp, quietly moving the goalposts on how long something’s been sitting dormant. Call promote_idea on a project that’s already three stages in, and it would reset it back to square one.

Adding the checks was the easy part

Four functions, four one-line guards, all using an error type that already existed. Reject the wrong starting status, return early, done. I wrote the guard, wrote a test that vaults something twice and checks the timestamp didn’t move, moved on to the next function.

The interesting one was the fifth function, the one the issue didn’t even mention.

The function that calls another function stopped being able to

Advancing a project to its next stage does two things: it logs a touch (you worked on this, that’s why it’s moving) and it moves the folder to the new stage’s directory. Those were two separate database commits today - log the touch, then, in a second step, move the stage. If the second step failed, the first one had already gone through. The project would sink to the bottom of the worklist as if you’d just worked on it, without actually going anywhere.

Fixing that meant combining both writes into one transaction. Simple enough, except the “log a touch” step wasn’t its own database write inline in this function - it was a call out to the dedicated touch-logging function, which opens and commits its own transaction internally. You can’t nest one SQLite transaction inside another on the same connection. So the moment I wrote a single transaction wrapping both operations, I couldn’t call that other function anymore. I had to reach in and copy its two individual database statements directly into the new combined block.

That’s where it got interesting: the touch-logging function was one of the four I’d just added a guard to. Calling it used to be the reason advancing a stage was safe from a bad starting status - if the item wasn’t in the right state, the touch call itself would refuse and stop everything. Once I stopped calling it and inlined its raw statements instead, that protection didn’t come along for the ride. It was never written down anywhere as “this function is safe because it delegates to that one.” It just happened to be true, until a plumbing change for an unrelated reason made it stop being true, silently.

The fix was to write the same guard a second time, directly in the function that could no longer borrow it from its neighbor. Which meant this function needed a check the original bug report never asked for, because the report was written before anyone had traced through what combining these two writes would actually require.

Testing a failure without lying to yourself about it

Part of this work meant proving that a failed folder move leaves everything untouched, and that a failed file write after a successful database insert doesn’t leak an orphaned row. Both need a way to reliably make a filesystem operation fail inside a test.

The obvious trick is making a folder read-only and trying to write into it. I’ve used that before and it works fine, until it doesn’t: if the test happens to run as root, permission bits stop mattering, the write succeeds anyway, and the test passes for completely the wrong reason. It looks green. It’s testing nothing.

The fix that actually holds regardless of who’s running the test: put the wrong kind of thing at the path instead of the wrong permissions. Want a directory-creation call to fail? Put a plain file there first - you can’t turn a file into a directory no matter who you are. Want a file write to fail? Put a directory there instead. Root doesn’t get a special exemption from “that’s not a directory.”

I got this wrong once in the process, in a way worth mentioning because it’s an easy trap: one operation in the code under test moves an entire folder into place with a rename, rather than writing directly into it. I put my blocking file straight at the destination, and the test failed - but for the wrong reason. Renaming a folder onto an existing, non-empty destination fails on its own, before the code path I actually wanted to test ever runs. The fix was to put the blocking file inside the folder being moved, so it travels along with the rename and only causes trouble once it lands where I actually wanted to interrupt something.

What stuck with me

The recurring shape here isn’t “add validation,” it’s “a function’s correctness sometimes depends on how it’s called, not just on what it does.” Combining two functions’ writes into one transaction is a plumbing change - it doesn’t look like it should touch behavior. But if one of those functions was quietly acting as a gatekeeper for the other, removing the call removes the gate, and nothing about the type system tells you that happened. The only way I caught it was by asking, function by function, “what would happen right now if I called this from a state it wasn’t expecting” - and that’s a question worth asking again every time a refactor changes who calls whom, not just once when the guards are first written.