From aed21409c0e66e901f788a93fe07e9670c504c34 Mon Sep 17 00:00:00 2001 From: Ada Date: Thu, 30 Apr 2026 11:18:07 -0400 Subject: [PATCH] Roll back vote_grants audit row on dispatch failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug introduced (in spirit) by #46 and made very visible by #48: when DispatchReward throws AFTER the audit/idempotency row was inserted, the player is permanently locked out of ever claiming that vote. Future sweeps see HasGrantForDate=true and short-circuit before retrying, even though no reward was ever actually granted. This always could have happened (e.g. SQLite glitch on AdjustPoints), but #48's VipGift dispatch made it likely — a misspelled template name in the Vote Rewards settings would silently lock out every voter on that provider. Fix: narrow try/catch around the DispatchReward call. On exception, delete the audit row via the new IVoteGrantRepository.DeleteByKey so the next sweep can retry once the admin fixes the misconfiguration. The outer try/catch (network errors during GetClaimStatus / MarkClaimed) is unaffected — those failure modes don't leave an orphaned row, so they don't need rollback. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Data/Repositories/VoteGrantRepository.cs | 18 ++++++++++++ .../Features/VoteRewardsFeature.cs | 28 +++++++++++++++---- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/src/KitsuneCommand/Data/Repositories/VoteGrantRepository.cs b/src/KitsuneCommand/Data/Repositories/VoteGrantRepository.cs index 417c13f..51e7b9c 100644 --- a/src/KitsuneCommand/Data/Repositories/VoteGrantRepository.cs +++ b/src/KitsuneCommand/Data/Repositories/VoteGrantRepository.cs @@ -26,6 +26,15 @@ public interface IVoteGrantRepository IEnumerable GetForPlayer(string steamId, int limit = 50); int GetTotalCount(); + + /// + /// Removes a grant row by its idempotency key. Used as a rollback when + /// the reward dispatch step throws AFTER the audit row was inserted — + /// without this, a misconfigured reward (e.g. wrong VIP-gift template + /// name) would lock the player out of ever claiming, because every + /// future sweep would short-circuit on HasGrantForDate. + /// + int DeleteByKey(string provider, string steamId, string voteDate); } public class VoteGrantRepository : IVoteGrantRepository @@ -89,5 +98,14 @@ public int GetTotalCount() using var conn = _db.CreateConnection(); return conn.ExecuteScalar("SELECT COUNT(*) FROM vote_grants"); } + + public int DeleteByKey(string provider, string steamId, string voteDate) + { + using var conn = _db.CreateConnection(); + return conn.Execute(@" + DELETE FROM vote_grants + WHERE provider = @Provider AND steam_id = @SteamId AND vote_date = @VoteDate", + new { Provider = provider, SteamId = steamId, VoteDate = voteDate }); + } } } diff --git a/src/KitsuneCommand/Features/VoteRewardsFeature.cs b/src/KitsuneCommand/Features/VoteRewardsFeature.cs index e235d5a..a8d4651 100644 --- a/src/KitsuneCommand/Features/VoteRewardsFeature.cs +++ b/src/KitsuneCommand/Features/VoteRewardsFeature.cs @@ -310,11 +310,29 @@ private async Task TryGrantOnceAsync( return result; } - // STEP 2: grant the actual reward. If this fails the audit row - // is still in place — admin can see the failure in the log and - // intervene manually rather than the player getting a silent - // double-grant on retry. - var grantedDescription = DispatchReward(steamId, playerName, cfg); + // STEP 2: grant the actual reward. If dispatch throws (e.g. an + // admin typo'd the VIP-gift template name), we MUST roll back + // the audit row — otherwise HasGrantForDate will short-circuit + // every future sweep and the player is permanently locked out + // of claiming, even after the admin fixes the misconfiguration. + // + // The narrow try/catch here ensures we only roll back on a + // dispatch failure, not on the broader try below (which catches + // network errors from STEP 0 / STEP 3 — those don't leave an + // orphaned audit row). + string grantedDescription; + try + { + grantedDescription = DispatchReward(steamId, playerName, cfg); + } + catch (Exception dispatchEx) + { + _grantRepo.DeleteByKey(provider.Key, steamId, voteDate); + Log.Warning($"[KitsuneCommand] VoteRewards: dispatch failed for {steamId} via {provider.Key} — rolled back audit row so next sweep can retry. Cause: {dispatchEx.Message}"); + result.Outcome = ClaimOutcome.Error; + result.Message = $"{provider.DisplayName}: dispatch failed — {dispatchEx.Message}"; + return result; + } // STEP 3: tell the listing site we delivered. Failure here is // not catastrophic — next status check returns "unclaimed" again,