fix(rsqueue): roll back a partially completed permit sweep

[jonyoder]

What

DatabaseQueueSweeperTask.Run could commit a partially completed permit sweep and report it to the caller as a clean one.

Run deletes every expired permit inside a single transaction, and the deferred CompleteTransaction(&err) reads the function's err variable to decide between COMMIT and ROLLBACK. The delete inside the loop was written with :=:

err := tx.QueuePermitDelete(ctx, permit.PermitId())

which declares a new err scoped to the if body instead of assigning to the one the deferred call watches. So a failure part-way through the loop returned with the outer err still nil, and the transaction committed.

The observable result: the permits deleted before the failure were released and made permanent, the remainder were left held, and nothing recorded that the sweep had not finished. Run has no error return, so the only signal was a slog.Debug line.

Fix

Assign to the outer err, so a failed delete rolls the whole sweep back. The sweep is idempotent and scheduled, so the next pass retries the same work — a rollback costs one interval, where the previous behaviour left the queue in a state no caller asked for.

Tests

TestSweepDeleteErrRollsBack asserts on the error handed to CompleteTransaction rather than on a delete count. That distinction is the whole point: the buggy and fixed versions attempt exactly the same deletes and both return at the first failure, so no count can tell them apart. The only observable difference is whether the partial result is kept.

To make that assertable, QueueTestStore.CompleteTransaction now records the error it was given; it was previously an empty method, so no test could see commit-versus-rollback at all. The field is additive and no existing test reads it.

TestSweepOkCommits pins the successful case beside it, so the new assertion cannot pass for the wrong reason — without it, a fixture that never swept anything would satisfy a rollback assertion trivially.

Verified by reverting only sweeper.go and re-running: TestSweepDeleteErrRollsBack fails with obtained = nil against the expected delete error, and the other three cases still pass.

Checks

• go test ./pkg/rsqueue/... — passes.
• go build ./... and go vet ./... — clean.
• ./scripts/fmt-check.sh, ./scripts/header-check.sh, ./scripts/test-wiring.sh — pass.
• go test ./... — pkg/rsnotify/listeners/postgrespgx times out locally because it needs a reachable postgres host; that is the just test-integration path and is untouched by this change.

Notes

I checked for the same shadowing elsewhere in the module. The only other CompleteTransaction(&err) user is examples/cmd/markdownRenderer/store/queue.go, which assigns rather than redeclares, and its QueuePop uses gorm's managed db.Transaction(func(tx) error), which is correct by construction. This was the only instance.

Worth considering separately: govet's shadow check is not enabled in .golangci.yml, and it catches this class directly. Enabling it would likely need a pass over existing findings, so it is not bundled here.

🤖 Generated with Claude Code