SQLRepo.Save upserts a content item and increments an optimistic-concurrency counter, Rev, so two callers who read the same item can't silently clobber each other. Read it at Rev = 3, save it, and the second writer to try the same thing gets a conflict instead of a lost update.
That part worked. What it never told you: after your own Save call succeeded, your own item still said Rev = 3. The database had already moved to 4.
The copy nobody saw
Save builds its own addressable copy of the item to attach timestamps to before writing:
cp := reflect.New(r.elemType)
src := reflect.ValueOf(item)
// ... dereference src ...
cp.Elem().Set(src)
rv := cp.Elem()UpdatedAt gets set on rv. The SQL increments rev in the database. The conflict check reads RowsAffected(). Every one of those touches rv, or the database, or the result of the statement, never src, the value the caller is still holding. A successful Save returned nil and changed nothing the caller could see.
Confirmed live, not theorized
PUT /signals/{slug} on our own self-hosted instance responded "Rev": 0. A re-read one line later showed "Rev": 1. That's the bug in one HTTP exchange: the write succeeded, and the response describing it was already wrong.
We'd actually fixed a *symptom* of this once before. A publish-via-PUT called Save twice, once unconditionally, once more after stamping PublishedAt, and the second call's conflict check failed every single time, because the first call had already advanced the stored Rev with no way for the second call to know it. The fix back then was to stop calling Save twice. It was the right fix for that call site. It left the actual defect, Save not telling its caller the truth, exactly where it was, waiting for the next caller shaped differently enough to hit it again.
RETURNING, not a second query
The Rev value is owned by the database, SET rev = table.rev + 1, so there was no way to know the new value in Go without asking. The obvious fix, read it back afterward, doubles your round trips on every write.
The actual fix: ask for it in the same statement.
INSERT INTO signals (...) VALUES (...)
ON CONFLICT (id) DO UPDATE SET rev = signals.rev + 1, ...
WHERE signals.rev = $old_rev
RETURNING revSwap ExecContext for QueryRowContext, scan the one column back, write it onto the caller's own struct. One round trip, same as before, just a Query instead of an Exec. The conflict case falls out for free too: sql.ErrNoRows on the scan is exactly what RowsAffected() == 0 used to tell you, just discovered a different way.
UpdatedAt and CreatedAt had the identical bug for a simpler reason: Smeldr computes those values itself before the query even runs, so writing them back to the caller needed no round trip at all, just an assignment Save was never making.
The tests that passed for the wrong reason
Two existing tests proved the conflict-detection path still worked, by accident, using the very bug this fix removes. Both called Save twice on the same variable and relied on that variable's Rev never advancing to manufacture a stale write on the third call. Once Save started writing back, the second call correctly advanced the caller's Rev, and the third call would have quietly started succeeding instead of failing, and the test would go on passing, and prove nothing.
The fix: hold a second, deliberately separate copy that's never updated, instead of leaning on a variable that happened to stay stale for the wrong reason. It's a small rewrite, and it's the difference between a test that checks a contract and a test that checks whether a bug is still there.
What's still open
Save telling the truth to its own caller is one layer. A future caller that constructs its own update payload and simply leaves Rev out of it is a different, still-open problem: Smeldr's own headless-automation module already documents that as a contract the caller must uphold, not something the storage layer can enforce for you. This fix makes the honest path honest. It doesn't stop you from lying to yourself on purpose.