Skip to main content

max / makenotwork

Cover the pending-refund crash window A refund's claim and its completion are separate timestamps on purpose: the gap between them is a crash window, and which side of it a row sits on decides whether it may be retried automatically or has to reach a person. Nothing asserted that. The money path's most consequential branch, that a matched-but-incomplete refund is escalated rather than re-issued, was carried entirely by a comment (PAY-S1). Three tests: a claim records the match and not the completion and cannot be claimed twice; a released claim is re-claimable and is the same row; and the stale sweep surfaces both unfinished shapes, never-matched and matched-but-incomplete, while leaving a completed refund alone and not re-alerting an escalated one. Checked by narrowing the sweep to `matched_at IS NULL`, which drops the crash-window row and fails the third test. Not covered, and not coverable with the current mocks: the graceful- failure branch that releases a claim fires when `handle_charge_refunded` errors, and that function is pure database work, so the fault seam is the pool rather than any of the three mocks the harness can fail. Phase 1 of wiki `testing-posture`.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-08-05 14:21 UTC
Signed with PGP, not checked
Commit: b591ef01154c7ed0485d7a7f785bfc696a893494
Parent: 90c96f8
1 file changed, +161 insertions, -0 deletions
M server/tests/workflows/failure_paths.rs +161
@@ -705,3 +705,164 @@
705 705 "and the tracking row is still cleared"
706 706 );
707 707 }
708 +
709 + // The pending-refund crash window (PAY-S1)
710 +
711 + /// A refund's claim is deliberately not the same thing as its completion. The
712 + /// gap between them is a crash window, and which side of it a row is on decides
713 + /// whether the row may be retried automatically or must reach a human. These
714 + /// assert that distinction, which is the whole reason the two timestamps are
715 + /// separate columns.
716 + ///
717 + /// The graceful-failure branch in `check_pending_refund` (release the claim so a
718 + /// later delivery retries) is NOT covered here: it fires when
719 + /// `handle_charge_refunded` returns an error, and that function is pure database
720 + /// work, so the seam is the pool rather than any mock the fault harness reaches.
721 + use makenotwork::db::Cents;
722 + use makenotwork::db::pending_refunds;
723 +
724 + async fn refund_row(h: &TestHarness, pi: &str) -> (bool, bool, bool) {
725 + sqlx::query_as(
726 + "SELECT matched_at IS NOT NULL, completed_at IS NOT NULL, escalated_at IS NOT NULL
727 + FROM pending_refunds WHERE payment_intent_id = $1",
728 + )
729 + .bind(pi)
730 + .fetch_one(&h.db)
731 + .await
732 + .unwrap()
733 + }
734 +
735 + /// A claim marks the row matched and nothing else. Recording completion at claim
736 + /// time would erase the crash window: a process killed mid-refund would look
737 + /// handled, and the refund would be silently dropped.
738 + #[tokio::test]
739 + async fn claiming_a_refund_does_not_record_it_as_completed() {
740 + let h = TestHarness::new().await;
741 + let pi = "pi_claim_only";
742 + pending_refunds::insert_pending_refund(&h.db, pi, 1000, 1000)
743 + .await
744 + .unwrap();
745 +
746 + let claimed = pending_refunds::claim_pending_refund(&h.db, pi)
747 + .await
748 + .unwrap()
749 + .expect("the row is unmatched, so it claims");
750 + assert_eq!(claimed.amount, Cents::new(1000));
751 +
752 + let (matched, completed, _) = refund_row(&h, pi).await;
753 + assert!(matched, "the claim is recorded");
754 + assert!(
755 + !completed,
756 + "completion must wait for the refund work to succeed"
757 + );
758 +
759 + assert!(
760 + pending_refunds::claim_pending_refund(&h.db, pi)
761 + .await
762 + .unwrap()
763 + .is_none(),
764 + "a claimed refund must not be claimable twice, that would double-refund"
765 + );
766 +
767 + pending_refunds::mark_refund_completed(&h.db, claimed.id)
768 + .await
769 + .unwrap();
770 + let (_, completed, _) = refund_row(&h, pi).await;
771 + assert!(completed, "completion is recorded separately");
772 + }
773 +
774 + /// A graceful failure releases the claim, and the released row must be claimable
775 + /// again. Without the re-claim the release accomplishes nothing.
776 + #[tokio::test]
777 + async fn releasing_a_claim_reopens_the_refund_for_retry() {
778 + let h = TestHarness::new().await;
779 + let pi = "pi_released";
780 + pending_refunds::insert_pending_refund(&h.db, pi, 500, 500)
781 + .await
782 + .unwrap();
783 +
784 + let first = pending_refunds::claim_pending_refund(&h.db, pi)
785 + .await
786 + .unwrap()
787 + .unwrap();
788 + pending_refunds::unclaim_pending_refund(&h.db, first.id)
789 + .await
790 + .unwrap();
791 +
792 + let (matched, completed, _) = refund_row(&h, pi).await;
793 + assert!(!matched, "the release clears the claim");
794 + assert!(!completed, "and it is still not complete");
795 +
796 + let second = pending_refunds::claim_pending_refund(&h.db, pi)
797 + .await
798 + .unwrap()
799 + .expect("a released refund must be re-claimable");
800 + assert_eq!(second.id, first.id, "the same row, retried");
801 + }
802 +
803 + /// The sweep's whole job is to catch what neither the webhook nor the retry
804 + /// caught. Both shapes of unfinished refund must surface: never matched, and
805 + /// matched-but-incomplete (the process died mid-refund, PAY-S1). A completed one
806 + /// must not, or every settled refund would be escalated to a human forever.
807 + #[tokio::test]
808 + async fn the_stale_sweep_surfaces_both_unfinished_shapes_and_not_completed_ones() {
809 + let h = TestHarness::new().await;
810 + for (pi, amount) in [("pi_never", 100), ("pi_crashed", 200), ("pi_done", 300)] {
811 + pending_refunds::insert_pending_refund(&h.db, pi, amount, amount)
812 + .await
813 + .unwrap();
814 + }
815 + // Age them all past the sweep's window.
816 + sqlx::query("UPDATE pending_refunds SET created_at = NOW() - INTERVAL '48 hours'")
817 + .execute(&h.db)
818 + .await
819 + .unwrap();
820 +
821 + // pi_crashed: claimed, then the process died before completion.
822 + let crashed = pending_refunds::claim_pending_refund(&h.db, "pi_crashed")
823 + .await
824 + .unwrap()
825 + .unwrap();
826 + // pi_done: claimed and completed, the settled case.
827 + let done = pending_refunds::claim_pending_refund(&h.db, "pi_done")
828 + .await
829 + .unwrap()
830 + .unwrap();
831 + pending_refunds::mark_refund_completed(&h.db, done.id)
832 + .await
833 + .unwrap();
834 +
835 + let stale = pending_refunds::get_stale_refunds(&h.db, chrono::Duration::hours(24))
836 + .await
837 + .unwrap();
838 + let ids: Vec<&str> = stale.iter().map(|r| r.payment_intent_id.as_str()).collect();
839 +
840 + assert!(
841 + ids.contains(&"pi_never"),
842 + "a refund that never matched a payment needs attention"
843 + );
844 + assert!(
845 + ids.contains(&"pi_crashed"),
846 + "matched-but-incomplete is the crash window and must reach a human, \
847 + it is deliberately not auto-retried because re-issuing could double-refund"
848 + );
849 + assert!(
850 + !ids.contains(&"pi_done"),
851 + "a completed refund is settled and must not be escalated"
852 + );
853 +
854 + // Escalation is idempotent: an escalated row stops being surfaced, so the
855 + // sweep alerts once rather than every tick until someone acts.
856 + pending_refunds::mark_escalated(&h.db, crashed.id)
857 + .await
858 + .unwrap();
859 + let after = pending_refunds::get_stale_refunds(&h.db, chrono::Duration::hours(24))
860 + .await
861 + .unwrap();
862 + assert!(
863 + !after.iter().any(|r| r.payment_intent_id == "pi_crashed"),
864 + "an escalated refund must not be re-alerted every tick"
865 + );
866 + let (_, _, escalated) = refund_row(&h, "pi_crashed").await;
867 + assert!(escalated, "and the escalation is recorded on the row");
868 + }