fix(core): restore refund sync for terminal statuses when force_sync is set - #13723
Open
lorenzozanee wants to merge 1 commit into
Open
fix(core): restore refund sync for terminal statuses when force_sync is set#13723lorenzozanee wants to merge 1 commit into
lorenzozanee wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type of Change
Description
GET /refunds/{id}?force_sync=truesilently no-ops when the refund is already in a terminal status (Success/Failure):should_call_refundgatesforce_syncbehind a non-terminal-status check, so the request returns the stale database row without consulting the connector.This restores the pre-#11725 behavior and v2 parity:
force_syncis again an unconditional override that consults the connector even for terminal refunds, andTransactionFailureis now treated as a terminal failure state likeFailure. When the connector is called,sync_refund_with_gatewayalready materializes the returned status (including a connector-side reversal) and fires the outgoing webhook — only the predicate was blocking it.Note: with this change, non-terminal refunds (
Pending,ManualReview) are again synced with the connector when retrieved withoutforce_sync, matching v2 and the behavior before #11725.Additional Changes
Motivation and Context
Fixes #13680
Since #11725 (released 2026.04.10.0), an explicit
force_sync=trueon a refund inSuccess/Failurereturns the stale row — no connector call, no error, no indication the sync was skipped. If the connector later reverses a succeeded refund and the reversal webhook is lost, the refund stays stuck atsuccesswith no supported way to pull the connector's truth back in. v2 still has the unconditionalforce_syncdisjunct, so the same API behaves oppositely between v1 and v2.#11725's own description says terminal refunds should skip sync when "all_keys_required or force_sync" is set, but the merged code only gates
force_sync— leaving theall_keys_requiredbypass and contradicting its stated intent. The terminal set also omitsTransactionFailure, which the rest of the refund flow already treats as terminal.Note: v1 no longer has the
connector_refund_idguard that v2 still has (removed in #12755), so v1 may consult the connector for a refund without a connector refund id; that v1/v2 divergence is tracked separately and deliberately left out of this change.How did you test it?
should_call_refundcovering all fiveRefundStatusvariants ×force_sync×all_keys_required(20 cases), including the regression direction terminal +force_sync=true.force_sync=truereturns false) and passes with the fix:cargo test -p router --lib --features v1 core::refunds::tests::should_call_refund_predicate_directions→ ok, 1 passed.Checklist
cargo +nightly fmt --allcargo clippy