From 85d688403cf1a3b4f9081816804e790f84b8d497 Mon Sep 17 00:00:00 2001
From: Astrid Gealer <astrid@gealer.email>
Date: Mon, 26 Oct 2026 20:47:21 +0000
Subject: [PATCH 1/2] fix(config/db): compare-and-set the revocation in-flight
 flip; make 000059 re-applicable after a rollback

FlipPurchaseRevocationInFlight was an unconditional UPDATE, so a stale
concurrent Azure revoke reaching it after another request's
MarkPurchaseRevoked set revocation_in_flight on a revoked row, where
nothing clears it. The flip now only matches revoked_at IS NULL; a
revoked row returns ErrPurchaseAlreadyRevoked (a missing row keeps its
not-found error), and the handler answers already_revoked without
posting a second Azure Return.

Rolling back through 000059 and re-applying it failed: 000064's down is
a no-op, so Purchaser stays at ...0007 and 000059's guard refused it as
a foreign "Purchaser". 000059 now yields to that row (and restores its
system_managed flag), and its admin backfill only targets ...0005 when
that row really is Purchaser, so a re-apply no longer adds admins to
Standard Users. Adds a 000059 down/up round-trip test and a
down-to-version-1 round trip. No new migration.

Refs #316

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 internal/api/handler_purchases_revoke.go      |  39 +++++--
 .../handler_purchases_revoke_guard_test.go    |  56 ++++++++++
 internal/config/errors.go                     |   6 ++
 internal/config/interfaces.go                 |   4 +-
 internal/config/store_postgres.go             |  26 ++++-
 ...tore_postgres_issue316_integration_test.go |  86 +++++++++++++++
 .../000059_seed_purchaser_group.down.sql      |   7 +-
 .../000059_seed_purchaser_group.up.sql        |  35 ++++--
 .../000059_seed_purchaser_group_test.go       | 101 ++++++++++++++++++
 .../migrations/migrate_down_all_test.go       |  21 ++++
 10 files changed, 359 insertions(+), 22 deletions(-)

diff --git a/internal/api/handler_purchases_revoke.go b/internal/api/handler_purchases_revoke.go
index a5ae9b027..d48c850f8 100644
--- a/internal/api/handler_purchases_revoke.go
+++ b/internal/api/handler_purchases_revoke.go
@@ -787,13 +787,8 @@ func (h *Handler) callAzureReturn(
 		return nil, quoteErr
 	}
 
-	// Partial-success guard (issue #290 Finding #6): flip the in-flight flag
-	// BEFORE calling Azure Return so that if the subsequent MarkPurchaseRevoked
-	// write fails, the row is visible to the finalize_revocations sweep rather
-	// than silently stuck. Best-effort: if the flip itself fails, log and
-	// continue — the in-flight flag is a safety net, not a hard precondition.
-	if flipErr := h.config.FlipPurchaseRevocationInFlight(ctx, record.PurchaseID); flipErr != nil {
-		logging.Warnf("revoke azure: FlipPurchaseRevocationInFlight for %s failed (continuing): %v", record.PurchaseID, flipErr)
+	if revoked := h.flipRevocationInFlight(ctx, record); revoked != nil {
+		return revoked, nil
 	}
 
 	// Step 2: Return (post the actual refund request).
@@ -815,6 +810,36 @@ func (h *Handler) callAzureReturn(
 	return h.persistAzureRevocation(ctx, record, calcRefundAmount, calcRefundCurrency)
 }
 
+// flipRevocationInFlight sets the in-flight flag BEFORE the Azure Return
+// (partial-success guard, issue #290 Finding #6) so that if the subsequent
+// MarkPurchaseRevoked write fails, the row is visible to the
+// finalize_revocations sweep rather than silently stuck. Best-effort: if the
+// flip itself fails, log and continue — the in-flight flag is a safety net,
+// not a hard precondition.
+//
+// The one exception is a row the flip found already revoked (issue #316): a
+// concurrent request finished the revocation after this one loaded the row,
+// so no second Return is posted and the caller answers already_revoked, as
+// the up-front idempotency check would have. The result is nil otherwise.
+func (h *Handler) flipRevocationInFlight(ctx context.Context, record *config.PurchaseHistoryRecord) *revokePurchaseResult {
+	flipErr := h.config.FlipPurchaseRevocationInFlight(ctx, record.PurchaseID)
+	if flipErr == nil {
+		return nil
+	}
+	if !errors.Is(flipErr, config.ErrPurchaseAlreadyRevoked) {
+		logging.Warnf("revoke azure: FlipPurchaseRevocationInFlight for %s failed (continuing): %v", record.PurchaseID, flipErr)
+		return nil
+	}
+	logging.Infof("revoke azure: purchase_id=%s was revoked by a concurrent request; not posting a second Return", record.PurchaseID)
+	result := &revokePurchaseResult{Status: "already_revoked"}
+	// Best-effort: the stamp only fills in the response.
+	if current, err := h.config.GetPurchaseHistoryByPurchaseID(ctx, record.PurchaseID); err == nil && current != nil && current.RevokedAt != nil {
+		result.RevokedAt = current.RevokedAt.Format(time.RFC3339)
+		result.RevokedVia = current.RevokedVia
+	}
+	return result
+}
+
 // checkConfirmedRefund is the TOCTOU-divergence check: the refund the user
 // confirmed must match Azure's current CalculateRefund quote in currency and,
 // within revokeQuoteEpsilon, in amount. A mismatch means the quote changed
diff --git a/internal/api/handler_purchases_revoke_guard_test.go b/internal/api/handler_purchases_revoke_guard_test.go
index 003cf5cf2..84fa3fdcd 100644
--- a/internal/api/handler_purchases_revoke_guard_test.go
+++ b/internal/api/handler_purchases_revoke_guard_test.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"testing"
+	"time"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
 	armreservations "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations"
@@ -177,3 +178,58 @@ func TestRevokePurchase_AzureRefundGuard(t *testing.T) {
 func confirmedQuote(amount float64, currency string) revokeConfirmBody {
 	return revokeConfirmBody{ExpectedRefundAmount: &amount, ExpectedRefundCurrency: currency}
 }
+
+// TestCallAzureReturn_FlipFindsRowRevoked covers issue #316: the in-flight
+// flip is now a compare-and-set on revoked_at IS NULL. When it reports the
+// row already revoked, a concurrent request finished the revocation after
+// this one loaded the row, so no second Return is posted and the answer is
+// already_revoked. Any other flip failure stays best-effort.
+func TestCallAzureReturn_FlipFindsRowRevoked(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	quote := &armreservations.Price{Amount: toPtr(10.0), CurrencyCode: toPtr("USD")}
+
+	t.Run("already revoked: no Return, already_revoked", func(t *testing.T) {
+		t.Parallel()
+		mockStore := new(MockConfigStore)
+		t.Cleanup(func() { mockStore.AssertExpectations(t) })
+		r := armReservationRecord()
+		revokedAt := time.Date(2026, 10, 26, 12, 0, 0, 0, time.UTC)
+		mockStore.On("FlipPurchaseRevocationInFlight", ctx, r.PurchaseID).
+			Return(fmt.Errorf("FlipPurchaseRevocationInFlight: purchase_id %q: %w", r.PurchaseID, config.ErrPurchaseAlreadyRevoked))
+		current := *r
+		current.RevokedAt = &revokedAt
+		current.RevokedVia = "direct-api"
+		mockStore.On("GetPurchaseHistoryByPurchaseID", ctx, r.PurchaseID).Return(&current, nil)
+		returnClient := &stubReturnClient{}
+
+		h := &Handler{config: mockStore}
+		result, err := h.callAzureReturn(ctx, &countingCalcRefundClient{price: quote}, returnClient, r, "order-abc", "res-xyz", confirmedQuote(10, "USD"))
+		require.NoError(t, err)
+		assert.Zero(t, returnClient.calls, "a revoked row must not get a second Return")
+		res, ok := result.(*revokePurchaseResult)
+		require.True(t, ok, "got %T", result)
+		assert.Equal(t, "already_revoked", res.Status)
+		assert.Equal(t, revokedAt.Format(time.RFC3339), res.RevokedAt)
+		assert.Equal(t, "direct-api", res.RevokedVia)
+		mockStore.AssertNotCalled(t, "MarkPurchaseRevoked", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+	})
+
+	t.Run("other flip error: still returns and marks", func(t *testing.T) {
+		t.Parallel()
+		mockStore := new(MockConfigStore)
+		t.Cleanup(func() { mockStore.AssertExpectations(t) })
+		r := armReservationRecord()
+		mockStore.On("FlipPurchaseRevocationInFlight", ctx, r.PurchaseID).Return(fmt.Errorf("FlipPurchaseRevocationInFlight: connection reset"))
+		mockStore.On("MarkPurchaseRevoked", ctx, r.PurchaseID, mock.AnythingOfType("time.Time"), "direct-api", "", mock.Anything, mock.Anything).Return(nil)
+		returnClient := &stubReturnClient{}
+
+		h := &Handler{config: mockStore}
+		result, err := h.callAzureReturn(ctx, &countingCalcRefundClient{price: quote}, returnClient, r, "order-abc", "res-xyz", confirmedQuote(10, "USD"))
+		require.NoError(t, err)
+		assert.Equal(t, 1, returnClient.calls)
+		res, ok := result.(*revokePurchaseResult)
+		require.True(t, ok, "got %T", result)
+		assert.Equal(t, "revoked", res.Status)
+	})
+}
diff --git a/internal/config/errors.go b/internal/config/errors.go
index cd60fa293..691f86bbc 100644
--- a/internal/config/errors.go
+++ b/internal/config/errors.go
@@ -12,6 +12,12 @@ var ErrNotFound = errors.New("not found")
 // so an empty ID would be silently dropped after the first one (#774).
 var ErrEmptyPurchaseID = errors.New("purchase history record has no purchase id")
 
+// ErrPurchaseAlreadyRevoked is returned (wrapped) by
+// FlipPurchaseRevocationInFlight when the purchase_history row exists but is
+// already revoked: a concurrent revoke finished first, so the caller must not
+// start another provider return (issue #316).
+var ErrPurchaseAlreadyRevoked = errors.New("purchase is already revoked")
+
 // ErrPurchasePlanConflict means a plan changed after the caller read it.
 var ErrPurchasePlanConflict = errors.New("purchase plan changed")
 
diff --git a/internal/config/interfaces.go b/internal/config/interfaces.go
index 4046e1969..6efba46fc 100644
--- a/internal/config/interfaces.go
+++ b/internal/config/interfaces.go
@@ -415,7 +415,9 @@ type StoreInterface interface {
 	// call so that the row can be identified by the finalize sweep if the
 	// subsequent MarkPurchaseRevoked DB write fails (partial-success reconciliation,
 	// issue #290 Finding #6, migration 000072). No-op when the flag is already
-	// true (idempotent). Returns a not-found error when no row matches.
+	// true (idempotent). Only an unrevoked row is flipped: a revoked one
+	// returns an error wrapping ErrPurchaseAlreadyRevoked (issue #316).
+	// Returns a not-found error when no row matches.
 	FlipPurchaseRevocationInFlight(ctx context.Context, purchaseID string) error
 
 	// ClearRevocationInFlight resets revocation_in_flight=false on a
diff --git a/internal/config/store_postgres.go b/internal/config/store_postgres.go
index 2d6ade951..83357b4fd 100644
--- a/internal/config/store_postgres.go
+++ b/internal/config/store_postgres.go
@@ -2974,21 +2974,39 @@ func (s *PostgresStore) MarkPurchaseRevoked(ctx context.Context, purchaseID stri
 // FlipPurchaseRevocationInFlight sets revocation_in_flight=true on the
 // purchase_history row for purchaseID. Called immediately before the Azure
 // Return API call to enable partial-success reconciliation (issue #290
-// Finding #6). Idempotent: already-true rows are not modified. Returns a
-// not-found error when no row matches.
+// Finding #6). Idempotent: an unrevoked row whose flag is already true is
+// left as it is.
+//
+// Compare-and-set on revoked_at IS NULL (issue #316): only an unrevoked row
+// can be flipped. A stale concurrent revoke request that reaches the flip
+// after another request's MarkPurchaseRevoked committed would otherwise set
+// the flag again on a revoked row, which MarkPurchaseRevoked (guarded on the
+// same predicate) then never clears. The row lock serializes the two
+// UPDATEs, and the flip re-checks the predicate on the committed row, so the
+// flag ends false whichever lands first. A revoked row returns an error
+// wrapping ErrPurchaseAlreadyRevoked; a missing row keeps the not-found
+// error.
 func (s *PostgresStore) FlipPurchaseRevocationInFlight(ctx context.Context, purchaseID string) error {
 	tag, err := s.db.Exec(ctx, `
 		UPDATE purchase_history
 		   SET revocation_in_flight = true
 		 WHERE purchase_id = $1
+		   AND revoked_at IS NULL
 	`, purchaseID)
 	if err != nil {
 		return fmt.Errorf("FlipPurchaseRevocationInFlight: %w", err)
 	}
-	if tag.RowsAffected() == 0 {
+	if tag.RowsAffected() > 0 {
+		return nil
+	}
+	var exists bool
+	if err := s.db.QueryRow(ctx, `SELECT EXISTS(SELECT 1 FROM purchase_history WHERE purchase_id = $1)`, purchaseID).Scan(&exists); err != nil {
+		return fmt.Errorf("FlipPurchaseRevocationInFlight existence check: %w", err)
+	}
+	if !exists {
 		return fmt.Errorf("FlipPurchaseRevocationInFlight: purchase_id %q not found", purchaseID)
 	}
-	return nil
+	return fmt.Errorf("FlipPurchaseRevocationInFlight: purchase_id %q: %w", purchaseID, ErrPurchaseAlreadyRevoked)
 }
 
 // ClearRevocationInFlight sets revocation_in_flight=false on a purchase_history
diff --git a/internal/config/store_postgres_issue316_integration_test.go b/internal/config/store_postgres_issue316_integration_test.go
index 8eb6c732f..0a84b1abb 100644
--- a/internal/config/store_postgres_issue316_integration_test.go
+++ b/internal/config/store_postgres_issue316_integration_test.go
@@ -6,6 +6,7 @@ package config
 
 import (
 	"context"
+	"fmt"
 	"sync"
 	"testing"
 	"time"
@@ -121,6 +122,91 @@ func TestMarkPurchaseRevoked_ClearsRevocationInFlight(t *testing.T) {
 	})
 }
 
+// TestFlipPurchaseRevocationInFlight_CompareAndSet covers the follow-up to
+// A04-013 tracked on issue #316: the flip was an unconditional UPDATE, so a
+// stale revoke request reaching it after another request's
+// MarkPurchaseRevoked set the flag again on a revoked row, and
+// MarkPurchaseRevoked (guarded on revoked_at IS NULL) never cleared it.
+func TestFlipPurchaseRevocationInFlight_CompareAndSet(t *testing.T) {
+	ctx := context.Background()
+	store := setupRampStepStore(ctx, t)
+
+	t.Run("a revoked row is refused and keeps the flag clear", func(t *testing.T) {
+		const id = "azure-flip-316-a"
+		seedRevocationRow(ctx, t, store, id)
+		require.NoError(t, store.FlipPurchaseRevocationInFlight(ctx, id))
+		require.NoError(t, store.MarkPurchaseRevoked(ctx, id, time.Now().UTC(), "direct-api", "", nil, ""))
+
+		err := store.FlipPurchaseRevocationInFlight(ctx, id)
+		require.ErrorIs(t, err, ErrPurchaseAlreadyRevoked)
+		assert.NotContains(t, err.Error(), "not found")
+
+		got, err := store.GetPurchaseHistoryByPurchaseID(ctx, id)
+		require.NoError(t, err)
+		require.NotNil(t, got.RevokedAt)
+		assert.False(t, got.RevocationInFlight, "a revoked row must not be flagged in flight again")
+		assert.Zero(t, inFlightIndexed(ctx, t, store, id))
+	})
+
+	t.Run("an unrevoked row still flips, idempotently", func(t *testing.T) {
+		const id = "azure-flip-316-b"
+		seedRevocationRow(ctx, t, store, id)
+		require.NoError(t, store.FlipPurchaseRevocationInFlight(ctx, id))
+		require.NoError(t, store.FlipPurchaseRevocationInFlight(ctx, id), "a second flip of an in-flight row is a no-op")
+		assert.Equal(t, 1, inFlightIndexed(ctx, t, store, id))
+	})
+
+	t.Run("a missing row is still not found", func(t *testing.T) {
+		err := store.FlipPurchaseRevocationInFlight(ctx, "azure-flip-316-missing")
+		require.ErrorContains(t, err, "not found")
+		assert.NotErrorIs(t, err, ErrPurchaseAlreadyRevoked)
+	})
+
+	// The race the guard closes: one request's MarkPurchaseRevoked against
+	// stale requests' flips on the same in-flight row. The row lock
+	// serializes them; a flip that waits on the mark re-checks
+	// revoked_at IS NULL on the committed row and matches nothing. Whatever
+	// the interleaving, the row ends revoked with the flag clear, and every
+	// flip either landed before the mark or reports the row revoked.
+	t.Run("stale flips racing a mark", func(t *testing.T) {
+		const rounds, flips = 20, 4
+		for round := range rounds {
+			id := fmt.Sprintf("azure-flip-316-race-%d", round)
+			seedRevocationRow(ctx, t, store, id)
+			require.NoError(t, store.FlipPurchaseRevocationInFlight(ctx, id))
+
+			errs := make([]error, flips+1)
+			start := make(chan struct{})
+			var wg sync.WaitGroup
+			for i := range flips + 1 {
+				wg.Add(1)
+				go func() {
+					defer wg.Done()
+					<-start
+					if i == 0 {
+						errs[i] = store.MarkPurchaseRevoked(ctx, id, time.Now().UTC(), "direct-api", "", nil, "")
+						return
+					}
+					errs[i] = store.FlipPurchaseRevocationInFlight(ctx, id)
+				}()
+			}
+			close(start)
+			wg.Wait()
+
+			require.NoError(t, errs[0], "round %d: the mark", round)
+			for i, err := range errs[1:] {
+				if err != nil {
+					require.ErrorIs(t, err, ErrPurchaseAlreadyRevoked, "round %d: flip %d", round, i)
+				}
+			}
+			got, err := store.GetPurchaseHistoryByPurchaseID(ctx, id)
+			require.NoError(t, err)
+			require.NotNil(t, got.RevokedAt, "round %d", round)
+			require.False(t, got.RevocationInFlight, "round %d: a flip that lost to the mark must not re-flag the revoked row", round)
+		}
+	})
+}
+
 // TestSetPlanAccounts_ProviderCheckFailsClosed covers the store-side guard of
 // A04-012. DerivePlanProviders skipped "provider:service" keys (the form
 // migration 000040 wrote) and legacy keys with no provider prefix, and an
diff --git a/internal/database/postgres/migrations/000059_seed_purchaser_group.down.sql b/internal/database/postgres/migrations/000059_seed_purchaser_group.down.sql
index 896ddcd9b..f030ad471 100644
--- a/internal/database/postgres/migrations/000059_seed_purchaser_group.down.sql
+++ b/internal/database/postgres/migrations/000059_seed_purchaser_group.down.sql
@@ -9,9 +9,10 @@
 -- Deleting it unconditionally destroyed Standard Users and stripped it from
 -- every member (issue #316, A04-010).
 --
--- Not reversed: 000059's admin backfill added ...0005 to every Administrators
--- member. Where ...0005 is Standard Users those memberships cannot be told
--- apart from deliberate ones, so they stay.
+-- Not reversed: before issue #316's fix, 000059's admin backfill added ...0005
+-- to every Administrators member. Where ...0005 is Standard Users those
+-- memberships cannot be told apart from deliberate ones, so they stay. The up
+-- now backfills only when ...0005 really is Purchaser.
 
 DO $$
 DECLARE
diff --git a/internal/database/postgres/migrations/000059_seed_purchaser_group.up.sql b/internal/database/postgres/migrations/000059_seed_purchaser_group.up.sql
index 0c66eec62..6db02e1ff 100644
--- a/internal/database/postgres/migrations/000059_seed_purchaser_group.up.sql
+++ b/internal/database/postgres/migrations/000059_seed_purchaser_group.up.sql
@@ -12,6 +12,12 @@
 ALTER TABLE groups ADD COLUMN IF NOT EXISTS system_managed BOOLEAN NOT NULL DEFAULT FALSE;
 
 -- Mark the four existing seed groups as system-managed.
+--
+-- Re-applying this migration after a rollback (issue #316): 000064 moved
+-- Purchaser to ...0007 and its down is a no-op, so after a rollback through
+-- this migration that row is still there, and this migration's down dropped
+-- the system_managed column. Restore the flag on it too. On a forward run
+-- ...0007 does not exist yet, so this matches nothing extra.
 UPDATE groups
 SET system_managed = TRUE
 WHERE id IN (
@@ -19,29 +25,40 @@ WHERE id IN (
     '00000000-0000-5000-8000-000000000002',
     '00000000-0000-5000-8000-000000000003',
     '00000000-0000-5000-8000-000000000004'
-);
+)
+   OR (id = '00000000-0000-5000-8000-000000000007' AND name = 'Purchaser');
 
 -- Fail closed if a pre-existing group already owns the name "Purchaser"
 -- under a different UUID. Without this guard the bare ON CONFLICT DO
 -- NOTHING below would silently skip the seed and leave
 -- DefaultPurchaserGroupID absent, breaking the admin-backfill UPDATE
 -- further down and every callsite that keys off the fixed UUID.
+-- The one exception is 000064's own Purchaser at ...0007, left behind by a
+-- rollback through this migration (issue #316): 000064 owns that row, so the
+-- seed below yields to it instead of refusing.
 DO $$
 BEGIN
     IF EXISTS (
         SELECT 1
         FROM groups
         WHERE name = 'Purchaser'
-          AND id <> '00000000-0000-5000-8000-000000000005'
+          AND id NOT IN (
+              '00000000-0000-5000-8000-000000000005',
+              '00000000-0000-5000-8000-000000000007'
+          )
     ) THEN
         RAISE EXCEPTION
             'migration 000059: a group named ''Purchaser'' already exists with a different id; rename it before applying this migration so the seeded id (00000000-0000-5000-8000-000000000005) can be created';
     END IF;
 END $$;
 
--- Insert the Purchaser group. Idempotent on the seeded id only -- a
--- name-collision on a different id is caught by the DO block above so
--- the seed never silently goes missing.
+-- Insert the Purchaser group. A name-collision on any id other than
+-- ...0005 and ...0007 is caught by the DO block above so the seed never
+-- silently goes missing. The bare ON CONFLICT covers the two remaining
+-- cases: ...0005 already taken (by Standard Users from 000057 on every real
+-- database, issue #942) and Purchaser already at ...0007 (a re-apply after a
+-- rollback, issue #316), where groups.name's UNIQUE constraint would
+-- otherwise abort the migration.
 INSERT INTO groups (id, name, description, permissions, allowed_accounts, system_managed)
 VALUES (
     '00000000-0000-5000-8000-000000000005',
@@ -59,13 +76,17 @@ VALUES (
     ARRAY['*'],
     TRUE
 )
-ON CONFLICT (id) DO NOTHING;
+ON CONFLICT DO NOTHING;
 
 -- Auto-assign every existing admin-group member to the Purchaser group
 -- so upgrade preserves current behavior. Admins can later remove
 -- themselves to enforce strict separation of duties.
 -- We drive off group membership (Administrators group UUID) rather than
 -- the legacy role column (which has been dropped in migration 000057).
+-- Only when ...0005 really is Purchaser: where 000057's Standard Users holds
+-- it, this backfill used to add every admin to Standard Users instead
+-- (issue #942), and on a re-apply after a rollback it would do so again for
+-- every admin created since (issue #316). 000064 backfills the real Purchaser.
 UPDATE users
 SET group_ids = ARRAY(
     SELECT DISTINCT unnest(
@@ -73,4 +94,4 @@ SET group_ids = ARRAY(
     )
 )
 WHERE '00000000-0000-5000-8000-000000000001'::UUID = ANY(COALESCE(group_ids, '{}'))
-  AND EXISTS (SELECT 1 FROM groups WHERE id = '00000000-0000-5000-8000-000000000005');
+  AND EXISTS (SELECT 1 FROM groups WHERE id = '00000000-0000-5000-8000-000000000005' AND name = 'Purchaser');
diff --git a/internal/database/postgres/migrations/000059_seed_purchaser_group_test.go b/internal/database/postgres/migrations/000059_seed_purchaser_group_test.go
index 18fdffa48..6e295c725 100644
--- a/internal/database/postgres/migrations/000059_seed_purchaser_group_test.go
+++ b/internal/database/postgres/migrations/000059_seed_purchaser_group_test.go
@@ -135,3 +135,104 @@ func TestMigration_SeedPurchaserGroup_Down(t *testing.T) {
 		assert.ElementsMatch(t, []string{seedGroupDownSlotTest}, queryGroupIDsByEmail(t, ctx, pool, only))
 	})
 }
+
+// groupsSnapshot is every group row, in id order, as "id|name|system_managed".
+func groupsSnapshot(ctx context.Context, t *testing.T, pool *pgxpool.Pool) []string {
+	t.Helper()
+	rows, err := pool.Query(ctx, `SELECT id::text || '|' || name || '|' || system_managed::text FROM groups ORDER BY id`)
+	require.NoError(t, err)
+	defer rows.Close()
+	var out []string
+	for rows.Next() {
+		var row string
+		require.NoError(t, rows.Scan(&row))
+		out = append(out, row)
+	}
+	require.NoError(t, rows.Err())
+	return out
+}
+
+// TestMigration_SeedPurchaserGroup_DownUpRoundTrip covers the round trip
+// tracked on issue #316: 000064's down is a no-op, so rolling back through
+// 000059 leaves Purchaser at ...0007, and re-applying 000059 refused because
+// a group named Purchaser already existed under an id other than ...0005.
+// The round trip must succeed and leave the groups, including the relocated
+// Purchaser's system_managed flag, and the users' memberships as they were:
+// an admin who is not in Standard Users must not be added to it by 000059's
+// admin backfill, which only ever meant Purchaser.
+func TestMigration_SeedPurchaserGroup_DownUpRoundTrip(t *testing.T) {
+	ctx := context.Background()
+
+	t.Run("Standard Users at ...0005", func(t *testing.T) {
+		const admin, standard = "admin-059-roundtrip@test.example", "standard-059-roundtrip@test.example"
+		const lateAdmin = "late-admin-059-roundtrip@test.example"
+		pool := pinBeforeDown(ctx, t, 64, map[string][]string{
+			admin:     {adminGroupIDForPurchaserTest, seedGroupDownSlotTest, purchaserGroupIDTest},
+			standard:  {seedGroupDownSlotTest},
+			lateAdmin: {adminGroupIDForPurchaserTest, purchaserGroupIDTest},
+		})
+		assertSeedPurchaserRoundTrip(ctx, t, pool)
+		assert.ElementsMatch(t, []string{adminGroupIDForPurchaserTest, seedGroupDownSlotTest, purchaserGroupIDTest}, queryGroupIDsByEmail(t, ctx, pool, admin))
+		assert.ElementsMatch(t, []string{seedGroupDownSlotTest}, queryGroupIDsByEmail(t, ctx, pool, standard))
+		assert.ElementsMatch(t, []string{adminGroupIDForPurchaserTest, purchaserGroupIDTest}, queryGroupIDsByEmail(t, ctx, pool, lateAdmin),
+			"the re-applied backfill must not add an admin to Standard Users")
+	})
+
+	// With ...0005 free, the re-applied seed no longer hits its id conflict
+	// and would insert a second "Purchaser", which groups.name's UNIQUE
+	// constraint refuses.
+	t.Run("...0005 free", func(t *testing.T) {
+		const admin = "admin-059-roundtrip-free@test.example"
+		pool := pinBeforeDown(ctx, t, 64, map[string][]string{
+			admin: {adminGroupIDForPurchaserTest, purchaserGroupIDTest},
+		})
+		_, err := pool.Exec(ctx, `DELETE FROM groups WHERE id = $1`, seedGroupDownSlotTest)
+		require.NoError(t, err)
+		assertSeedPurchaserRoundTrip(ctx, t, pool)
+		_, ok := groupName(ctx, t, pool, seedGroupDownSlotTest)
+		assert.False(t, ok, "the re-applied seed must not create a group at ...0005")
+		assert.ElementsMatch(t, []string{adminGroupIDForPurchaserTest, purchaserGroupIDTest}, queryGroupIDsByEmail(t, ctx, pool, admin))
+	})
+}
+
+// The re-apply yields only to 000064's Purchaser at ...0007. A "Purchaser"
+// under any other id is still someone else's group, and 000059 still refuses
+// rather than silently skip its seed.
+func TestMigration_SeedPurchaserGroup_ForeignPurchaserStillRefused(t *testing.T) {
+	ctx := context.Background()
+	pool := pinBeforeDown(ctx, t, 64, nil)
+	require.NoError(t, migrations.MigrateToVersion(ctx, pool, getMigrationsPath(), previousMigrationVersion(t, 59)))
+	_, err := pool.Exec(ctx, `DELETE FROM groups WHERE id = $1`, purchaserGroupIDTest)
+	require.NoError(t, err)
+	_, err = pool.Exec(ctx, `
+		INSERT INTO groups (id, name, description, permissions, allowed_accounts)
+		VALUES ('00000000-0000-4000-8000-000000000316', 'Purchaser', 'operator-made', '[]'::jsonb, ARRAY['*'])`)
+	require.NoError(t, err)
+
+	err = migrations.MigrateToVersion(ctx, pool, getMigrationsPath(), 64)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "migration 000059: a group named 'Purchaser' already exists with a different id")
+}
+
+// assertSeedPurchaserRoundTrip rolls a database pinned at 000064 back through
+// 000059 and forward again, and checks every group came back unchanged.
+func assertSeedPurchaserRoundTrip(ctx context.Context, t *testing.T, pool *pgxpool.Pool) {
+	t.Helper()
+	name, ok := groupName(ctx, t, pool, purchaserGroupIDTest)
+	require.True(t, ok)
+	require.Equal(t, "Purchaser", name, "precondition: 000064 put Purchaser at ...0007")
+	groupsBefore := groupsSnapshot(ctx, t, pool)
+
+	require.NoError(t, migrations.MigrateToVersion(ctx, pool, getMigrationsPath(), previousMigrationVersion(t, 59)),
+		"rolling back through 000059 must succeed")
+	name, ok = groupName(ctx, t, pool, purchaserGroupIDTest)
+	require.True(t, ok, "precondition: 000064's no-op down leaves Purchaser at ...0007")
+	require.Equal(t, "Purchaser", name)
+
+	require.NoError(t, migrations.MigrateToVersion(ctx, pool, getMigrationsPath(), 64),
+		"re-applying 000059 after its rollback must succeed")
+	_, dirty, err := migrations.GetMigrationVersion(ctx, pool, getMigrationsPath())
+	require.NoError(t, err)
+	require.False(t, dirty)
+	assert.Equal(t, groupsBefore, groupsSnapshot(ctx, t, pool), "the round trip must leave every group as it was")
+}
diff --git a/internal/database/postgres/migrations/migrate_down_all_test.go b/internal/database/postgres/migrations/migrate_down_all_test.go
index 96fece35b..bd17254e4 100644
--- a/internal/database/postgres/migrations/migrate_down_all_test.go
+++ b/internal/database/postgres/migrations/migrate_down_all_test.go
@@ -47,3 +47,24 @@ func TestMigrations_DownAllRoundTrip(t *testing.T) {
 	require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, latest),
 		"re-applying every migration after a full down must succeed")
 }
+
+// TestMigrations_DownToOneRoundTrip rolls back to version 1 rather than 0, so
+// the tables 000001 created survive with the rows later migrations seeded,
+// then re-applies everything. 000064's down is a no-op by design, so its
+// Purchaser at ...0007 is still there when 000059 re-runs, and 000059 refused
+// it as a foreign "Purchaser" (issue #316).
+func TestMigrations_DownToOneRoundTrip(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	migrationsPath := getMigrationsPath()
+	pool := testhelpers.RequireMigratedDatabase(ctx, t, migrationsPath).DB.Pool()
+	latest := migrationVersionsDesc(t)[0]
+
+	require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, 1), "roll back to version 1")
+	require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, latest),
+		"re-applying every migration after a rollback to version 1 must succeed")
+	version, dirty, err := migrations.GetMigrationVersion(ctx, pool, migrationsPath)
+	require.NoError(t, err)
+	require.False(t, dirty)
+	require.Equal(t, latest, version)
+}

From a81a3c35f906b5e81c671381b1d6c9fa86075103 Mon Sep 17 00:00:00 2001
From: Astrid Gealer <astrid@gealer.email>
Date: Mon, 26 Oct 2026 22:10:05 +0000
Subject: [PATCH 2/2] fix(revoke): make the Azure in-flight flip an exclusive
 claim so only one revoke posts a Return

FlipPurchaseRevocationInFlight let a row that was already in flight be
flipped again, so two first-time revokes of one Azure purchase could both
pass it and both post a Return. Azure refused the second, and that
request's error path cleared the flag while the first request's refund
was still in flight; if the first request's MarkPurchaseRevoked then
failed, the row was left neither revoked nor flagged, out of reach of
the finalize_revocations sweep.

- The flip is now a compare-and-set on revoked_at IS NULL AND
  revocation_in_flight = false. The loser gets the new
  config.ErrRevocationInFlight.
- callAzureReturn answers RECONCILE_PENDING (the up-front in-flight
  response) to the loser and posts no Return. Any other flip error now
  stops before the Return with a 500 instead of posting one unclaimed.
  So only the claim holder reaches the Return and its
  ClearRevocationInFlight only clears its own flag.
- OpenAPI: RECONCILE_PENDING also covers a concurrent revoke posting the
  return.

Tests: real-Postgres race of two first-time revokes through the handler
(exactly one Return, the other answers RECONCILE_PENDING, the flag
survives while the winner's Return is in flight); store race of 6
concurrent flips (exactly one claims); handler unit tests for the loser
and the flip-error path.

Closes #1518

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 internal/api/handler_purchases_revoke.go      |  85 ++++++---
 ...purchases_revoke_claim_integration_test.go | 179 ++++++++++++++++++
 .../handler_purchases_revoke_guard_test.go    |  49 ++++-
 internal/api/openapi.yaml                     |   7 +-
 internal/config/errors.go                     |   7 +
 internal/config/interfaces.go                 |  22 ++-
 internal/config/store_postgres.go             |  57 ++++--
 ...tore_postgres_issue316_integration_test.go |  70 ++++++-
 8 files changed, 405 insertions(+), 71 deletions(-)
 create mode 100644 internal/api/handler_purchases_revoke_claim_integration_test.go

diff --git a/internal/api/handler_purchases_revoke.go b/internal/api/handler_purchases_revoke.go
index d48c850f8..cf307d8d6 100644
--- a/internal/api/handler_purchases_revoke.go
+++ b/internal/api/handler_purchases_revoke.go
@@ -200,15 +200,13 @@ func (h *Handler) loadAndRevokePurchaseHistory(ctx context.Context, req *events.
 
 	// Partial-success reconciliation (issue #290 Finding #6): if the
 	// revocation_in_flight flag is set but revoked_at is still NULL, Azure
-	// already issued the refund but our DB write failed. Return 207 so the
-	// frontend does not retry the Azure call (which would fail with "already
-	// returned"). The finalize_revocations sweep will reconcile the row.
+	// already issued the refund but our DB write failed, or another request
+	// holds the claim and is posting the Return now (issue #1518). Return 207
+	// so the frontend does not retry the Azure call (which would fail with
+	// "already returned"). The finalize_revocations sweep will reconcile the
+	// row.
 	if record.RevocationInFlight {
-		return &revokeReconcilePendingResult{
-			Code:          "RECONCILE_PENDING",
-			AzureReturned: true,
-			Message:       "Refund already issued. We will reconcile your audit record shortly. Do not retry.",
-		}, nil
+		return revocationInFlightResult(), nil
 	}
 
 	// Parse the confirmed quote from the request body. Azure revocations
@@ -787,8 +785,8 @@ func (h *Handler) callAzureReturn(
 		return nil, quoteErr
 	}
 
-	if revoked := h.flipRevocationInFlight(ctx, record); revoked != nil {
-		return revoked, nil
+	if stop, claimErr := h.claimRevocationInFlight(ctx, record); stop != nil || claimErr != nil {
+		return stop, claimErr
 	}
 
 	// Step 2: Return (post the actual refund request).
@@ -810,30 +808,58 @@ func (h *Handler) callAzureReturn(
 	return h.persistAzureRevocation(ctx, record, calcRefundAmount, calcRefundCurrency)
 }
 
-// flipRevocationInFlight sets the in-flight flag BEFORE the Azure Return
+// revocationInFlightResult is the answer for a row whose revocation another
+// request has in flight: the RECONCILE_PENDING response, which tells the
+// frontend not to retry.
+func revocationInFlightResult() *revokeReconcilePendingResult {
+	return &revokeReconcilePendingResult{
+		Code:          "RECONCILE_PENDING",
+		AzureReturned: true,
+		Message:       "Refund already issued. We will reconcile your audit record shortly. Do not retry.",
+	}
+}
+
+// claimRevocationInFlight sets the in-flight flag BEFORE the Azure Return
 // (partial-success guard, issue #290 Finding #6) so that if the subsequent
 // MarkPurchaseRevoked write fails, the row is visible to the
-// finalize_revocations sweep rather than silently stuck. Best-effort: if the
-// flip itself fails, log and continue — the in-flight flag is a safety net,
-// not a hard precondition.
+// finalize_revocations sweep rather than silently stuck.
 //
-// The one exception is a row the flip found already revoked (issue #316): a
-// concurrent request finished the revocation after this one loaded the row,
-// so no second Return is posted and the caller answers already_revoked, as
-// the up-front idempotency check would have. The result is nil otherwise.
-func (h *Handler) flipRevocationInFlight(ctx context.Context, record *config.PurchaseHistoryRecord) *revokePurchaseResult {
+// The flip is also the claim that lets exactly one request post the Return
+// (issue #1518). Both results are nil only when this request won it; every
+// other outcome stops before the Return:
+//   - the row was revoked by a concurrent request after this one loaded it
+//     (issue #316): already_revoked, as the up-front idempotency check would
+//     have answered;
+//   - another request holds the claim: RECONCILE_PENDING, as the up-front
+//     in-flight check would have answered;
+//   - the flip failed: an error (500). No Return has been posted, so a retry
+//     is safe, whereas posting one without the claim could race the holder's
+//     and, if this request's MarkPurchaseRevoked then failed too, leave a
+//     refund that no in-flight flag points the sweep at.
+//
+// Because only the claim holder reaches the Return, the ClearRevocationInFlight
+// in handleAzureReturnError only ever clears this request's own flag.
+func (h *Handler) claimRevocationInFlight(ctx context.Context, record *config.PurchaseHistoryRecord) (any, error) {
 	flipErr := h.config.FlipPurchaseRevocationInFlight(ctx, record.PurchaseID)
-	if flipErr == nil {
-		return nil
-	}
-	if !errors.Is(flipErr, config.ErrPurchaseAlreadyRevoked) {
-		logging.Warnf("revoke azure: FlipPurchaseRevocationInFlight for %s failed (continuing): %v", record.PurchaseID, flipErr)
-		return nil
+	switch {
+	case flipErr == nil:
+		return nil, nil
+	case errors.Is(flipErr, config.ErrRevocationInFlight):
+		logging.Infof("revoke azure: purchase_id=%s is already being revoked by a concurrent request; not posting a second Return", record.PurchaseID)
+		return revocationInFlightResult(), nil
+	case errors.Is(flipErr, config.ErrPurchaseAlreadyRevoked):
+		logging.Infof("revoke azure: purchase_id=%s was revoked by a concurrent request; not posting a second Return", record.PurchaseID)
+		return h.alreadyRevokedResult(ctx, record.PurchaseID), nil
+	default:
+		return nil, fmt.Errorf("revoke azure: claim revocation of %s (no Return posted): %w", record.PurchaseID, flipErr)
 	}
-	logging.Infof("revoke azure: purchase_id=%s was revoked by a concurrent request; not posting a second Return", record.PurchaseID)
+}
+
+// alreadyRevokedResult answers already_revoked for a row a concurrent request
+// revoked. Best-effort: the re-read stamp only fills in the response.
+func (h *Handler) alreadyRevokedResult(ctx context.Context, purchaseID string) *revokePurchaseResult {
 	result := &revokePurchaseResult{Status: "already_revoked"}
-	// Best-effort: the stamp only fills in the response.
-	if current, err := h.config.GetPurchaseHistoryByPurchaseID(ctx, record.PurchaseID); err == nil && current != nil && current.RevokedAt != nil {
+	if current, err := h.config.GetPurchaseHistoryByPurchaseID(ctx, purchaseID); err == nil && current != nil && current.RevokedAt != nil {
 		result.RevokedAt = current.RevokedAt.Format(time.RFC3339)
 		result.RevokedVia = current.RevokedVia
 	}
@@ -896,7 +922,8 @@ func (h *Handler) azureCalculateRefund(ctx context.Context, calcClient azureCalc
 
 // handleAzureReturnError clears the in-flight flag (no refund was issued) and
 // maps the Return error to the right status: 422 on the 7-day window edge, 400
-// on other client errors, 500 otherwise.
+// on other client errors, 500 otherwise. The caller holds the claim from
+// claimRevocationInFlight, so the flag it clears is its own (issue #1518).
 func (h *Handler) handleAzureReturnError(ctx context.Context, record *config.PurchaseHistoryRecord, err error) error {
 	// Azure Return failed. Clear the in-flight flag so the row is not left in
 	// a permanently sticky state that would mislead the finalize_revocations
diff --git a/internal/api/handler_purchases_revoke_claim_integration_test.go b/internal/api/handler_purchases_revoke_claim_integration_test.go
new file mode 100644
index 000000000..d5aad168a
--- /dev/null
+++ b/internal/api/handler_purchases_revoke_claim_integration_test.go
@@ -0,0 +1,179 @@
+//go:build integration
+
+package api
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"sync"
+	"sync/atomic"
+	"testing"
+	"time"
+
+	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
+	armreservations "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations"
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
+	"github.com/aws/aws-lambda-go/events"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// revokeRequestKey tags each concurrent revoke's context with its index, so
+// the stub Return client knows which request is calling it.
+type revokeRequestKey struct{}
+
+// barrierCalcRefundClient answers CalculateRefund with a fixed quote, but
+// only once every party has called it. Both revokes have loaded the row (not
+// revoked, not in flight) by then, so they reach the in-flight flip together,
+// as two first-time revokes would.
+type barrierCalcRefundClient struct {
+	price   *armreservations.Price
+	parties int32
+	arrived atomic.Int32
+	ready   chan struct{}
+}
+
+func (c *barrierCalcRefundClient) Post(ctx context.Context, _ string, _ armreservations.CalculateRefundRequest, _ *armreservations.CalculateRefundClientPostOptions) (armreservations.CalculateRefundClientPostResponse, error) {
+	if c.arrived.Add(1) == c.parties {
+		close(c.ready)
+	}
+	select {
+	case <-c.ready:
+	case <-ctx.Done():
+		return armreservations.CalculateRefundClientPostResponse{}, ctx.Err()
+	}
+	return armreservations.CalculateRefundClientPostResponse{
+		CalculateRefundResponse: armreservations.CalculateRefundResponse{
+			Properties: &armreservations.RefundResponseProperties{
+				SessionID:           toPtr("s-claim-race"),
+				BillingRefundAmount: c.price,
+			},
+		},
+	}, nil
+}
+
+// raceReturnClient stands in for Azure's Return API. The first Return
+// succeeds, but only after the other request has finished, so the other
+// request runs its whole path while the first one's refund is in flight.
+// Azure refuses a second Return of the same reservation, so any later call
+// gets a 400. While the first Return is in flight, it records whether the row
+// still carries the in-flight flag.
+type raceReturnClient struct {
+	store      *config.PostgresStore
+	purchaseID string
+	done       [2]chan struct{}
+	calls      atomic.Int32
+	// flagHeld is whether the row was still flagged in flight once the other
+	// request had finished, while the first Return was still in flight.
+	flagHeld atomic.Bool
+}
+
+func (c *raceReturnClient) Post(ctx context.Context, _ string, _ armreservations.RefundRequest, _ *armreservations.ReturnClientPostOptions) (armreservations.ReturnClientPostResponse, error) {
+	if c.calls.Add(1) > 1 {
+		return armreservations.ReturnClientPostResponse{}, &azcore.ResponseError{StatusCode: 400, ErrorCode: "ReservationAlreadyReturned"}
+	}
+	me, _ := ctx.Value(revokeRequestKey{}).(int)
+	select {
+	case <-c.done[1-me]:
+	case <-time.After(10 * time.Second):
+		// The other request never finished; the assertions report it.
+	case <-ctx.Done():
+		return armreservations.ReturnClientPostResponse{}, ctx.Err()
+	}
+	row, err := c.store.GetPurchaseHistoryByPurchaseID(context.Background(), c.purchaseID)
+	if err == nil && row != nil {
+		c.flagHeld.Store(row.RevocationInFlight)
+	}
+	return armreservations.ReturnClientPostResponse{}, nil
+}
+
+// TestRevokeAzure_ConcurrentFirstRevokes_OneReturn covers issue #1518 on real
+// Postgres. Two first-time revokes of the same Azure purchase load the row
+// (unrevoked, not in flight) and reach the in-flight flip together. Before
+// the fix the flip let an already-flagged row be flipped again, so both
+// posted a Return; Azure refused the second, and that request's error
+// handling cleared the flag while the first request's refund was still in
+// flight. Had the first request's MarkPurchaseRevoked then failed, the row
+// would have been left neither revoked nor flagged, out of the
+// finalize_revocations sweep's reach.
+//
+// Now exactly one request posts a Return and records the revocation, the
+// other answers RECONCILE_PENDING without posting one, and the flag stays set
+// until the holder's MarkPurchaseRevoked clears it.
+func TestRevokeAzure_ConcurrentFirstRevokes_OneReturn(t *testing.T) {
+	_, store := setupSubmitDB(t)
+	const rounds = 5
+	for round := range rounds {
+		t.Run(fmt.Sprintf("round %d", round), func(t *testing.T) {
+			runConcurrentFirstRevokes(t, store, round)
+		})
+	}
+}
+
+func runConcurrentFirstRevokes(t *testing.T, store *config.PostgresStore, round int) {
+	ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
+	defer cancel()
+
+	record := withinWindowRecord(fmt.Sprintf(
+		"/providers/Microsoft.Capacity/reservationOrders/order-1518-%d/reservations/res-1518-%d", round, round))
+	record.AccountID = fmt.Sprintf("acct-1518-%d", round)
+	record.Region = "eastus"
+	record.ResourceType = "Standard_D2s_v3"
+	require.NoError(t, store.SavePurchaseHistory(ctx, record))
+
+	calc := &barrierCalcRefundClient{
+		price:   &armreservations.Price{Amount: toPtr(10.0), CurrencyCode: toPtr("USD")},
+		parties: 2,
+		ready:   make(chan struct{}),
+	}
+	ret := &raceReturnClient{store: store, purchaseID: record.PurchaseID, done: [2]chan struct{}{make(chan struct{}), make(chan struct{})}}
+	h := &Handler{config: store, azureRevokeFactory: &azureRevokeClientFactory{
+		newCredential:            func() (azcore.TokenCredential, error) { return nil, nil },
+		newCalculateRefundClient: func(azcore.TokenCredential) (azureCalculateRefundClient, error) { return calc, nil },
+		newReturnClient:          func(azcore.TokenCredential) (azureReturnClient, error) { return ret, nil },
+	}}
+	body, err := json.Marshal(confirmedQuote(10, "USD"))
+	require.NoError(t, err)
+	req := &events.LambdaFunctionURLRequest{Body: string(body)}
+
+	results := make([]any, 2)
+	errs := make([]error, 2)
+	var wg sync.WaitGroup
+	for i := range 2 {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			defer close(ret.done[i])
+			reqCtx := context.WithValue(ctx, revokeRequestKey{}, i)
+			results[i], errs[i] = h.loadAndRevokePurchaseHistory(reqCtx, req, revokeAdminSession(), record.PurchaseID)
+		}()
+	}
+	wg.Wait()
+
+	assert.Equal(t, int32(1), ret.calls.Load(), "exactly one of two concurrent first revokes may post an Azure Return")
+	assert.True(t, ret.flagHeld.Load(), "the other request must leave the in-flight flag set while the first one's Return is in flight")
+
+	revoked, inFlight := 0, 0
+	for i := range 2 {
+		require.NoError(t, errs[i], "request %d", i)
+		switch res := results[i].(type) {
+		case *revokePurchaseResult:
+			assert.Equal(t, "revoked", res.Status, "request %d", i)
+			revoked++
+		case *revokeReconcilePendingResult:
+			assert.Equal(t, revocationInFlightResult(), res, "request %d", i)
+			inFlight++
+		default:
+			t.Errorf("request %d: unexpected result %T %+v", i, results[i], results[i])
+		}
+	}
+	assert.Equal(t, 1, revoked, "one request revokes")
+	assert.Equal(t, 1, inFlight, "the other answers in flight (RECONCILE_PENDING)")
+
+	got, err := store.GetPurchaseHistoryByPurchaseID(ctx, record.PurchaseID)
+	require.NoError(t, err)
+	require.NotNil(t, got)
+	assert.NotNil(t, got.RevokedAt, "the winner's MarkPurchaseRevoked must land")
+	assert.False(t, got.RevocationInFlight, "MarkPurchaseRevoked clears the holder's flag")
+}
diff --git a/internal/api/handler_purchases_revoke_guard_test.go b/internal/api/handler_purchases_revoke_guard_test.go
index 84fa3fdcd..33ab05bbe 100644
--- a/internal/api/handler_purchases_revoke_guard_test.go
+++ b/internal/api/handler_purchases_revoke_guard_test.go
@@ -183,7 +183,9 @@ func confirmedQuote(amount float64, currency string) revokeConfirmBody {
 // flip is now a compare-and-set on revoked_at IS NULL. When it reports the
 // row already revoked, a concurrent request finished the revocation after
 // this one loaded the row, so no second Return is posted and the answer is
-// already_revoked. Any other flip failure stays best-effort.
+// already_revoked. Since issue #1518 the flip is also the claim that lets one
+// request post the Return: a row another request holds answers
+// RECONCILE_PENDING, and any other flip failure stops before the Return.
 func TestCallAzureReturn_FlipFindsRowRevoked(t *testing.T) {
 	t.Parallel()
 	ctx := context.Background()
@@ -215,21 +217,52 @@ func TestCallAzureReturn_FlipFindsRowRevoked(t *testing.T) {
 		mockStore.AssertNotCalled(t, "MarkPurchaseRevoked", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
 	})
 
-	t.Run("other flip error: still returns and marks", func(t *testing.T) {
+	// Issue #1518: another request holds the claim. This one must neither
+	// post a Return nor touch the flag (clearing it would orphan the holder's
+	// refund from the finalize sweep), and answers RECONCILE_PENDING like the
+	// up-front in-flight check.
+	t.Run("in flight: no Return, no clear, RECONCILE_PENDING", func(t *testing.T) {
 		t.Parallel()
 		mockStore := new(MockConfigStore)
 		t.Cleanup(func() { mockStore.AssertExpectations(t) })
 		r := armReservationRecord()
-		mockStore.On("FlipPurchaseRevocationInFlight", ctx, r.PurchaseID).Return(fmt.Errorf("FlipPurchaseRevocationInFlight: connection reset"))
-		mockStore.On("MarkPurchaseRevoked", ctx, r.PurchaseID, mock.AnythingOfType("time.Time"), "direct-api", "", mock.Anything, mock.Anything).Return(nil)
+		mockStore.On("FlipPurchaseRevocationInFlight", ctx, r.PurchaseID).
+			Return(fmt.Errorf("FlipPurchaseRevocationInFlight: purchase_id %q: %w", r.PurchaseID, config.ErrRevocationInFlight))
+		// Registered only so a regression fails on the assertions below
+		// rather than on an unexpected-call panic.
+		mockStore.On("MarkPurchaseRevoked", ctx, r.PurchaseID, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Maybe()
 		returnClient := &stubReturnClient{}
 
 		h := &Handler{config: mockStore}
 		result, err := h.callAzureReturn(ctx, &countingCalcRefundClient{price: quote}, returnClient, r, "order-abc", "res-xyz", confirmedQuote(10, "USD"))
 		require.NoError(t, err)
-		assert.Equal(t, 1, returnClient.calls)
-		res, ok := result.(*revokePurchaseResult)
-		require.True(t, ok, "got %T", result)
-		assert.Equal(t, "revoked", res.Status)
+		assert.Zero(t, returnClient.calls, "the request that loses the claim must not post a Return")
+		assert.Equal(t, revocationInFlightResult(), result)
+		mockStore.AssertNotCalled(t, "ClearRevocationInFlight", mock.Anything, mock.Anything)
+		mockStore.AssertNotCalled(t, "MarkPurchaseRevoked", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+	})
+
+	// Issue #1518: a request that could not establish the claim does not know
+	// whether another request holds it, so it posts no Return (a retry is
+	// safe) and leaves the flag alone.
+	t.Run("other flip error: no Return, no clear, 500", func(t *testing.T) {
+		t.Parallel()
+		mockStore := new(MockConfigStore)
+		t.Cleanup(func() { mockStore.AssertExpectations(t) })
+		r := armReservationRecord()
+		mockStore.On("FlipPurchaseRevocationInFlight", ctx, r.PurchaseID).Return(fmt.Errorf("FlipPurchaseRevocationInFlight: connection reset"))
+		mockStore.On("MarkPurchaseRevoked", ctx, r.PurchaseID, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Maybe()
+		returnClient := &stubReturnClient{}
+
+		h := &Handler{config: mockStore}
+		result, err := h.callAzureReturn(ctx, &countingCalcRefundClient{price: quote}, returnClient, r, "order-abc", "res-xyz", confirmedQuote(10, "USD"))
+		require.Error(t, err)
+		assert.Nil(t, result)
+		assert.ErrorContains(t, err, "connection reset")
+		_, isClientErr := IsClientError(err)
+		assert.False(t, isClientErr, "a store failure is a 500, not a client error")
+		assert.Zero(t, returnClient.calls, "no Return without the claim")
+		mockStore.AssertNotCalled(t, "ClearRevocationInFlight", mock.Anything, mock.Anything)
+		mockStore.AssertNotCalled(t, "MarkPurchaseRevoked", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
 	})
 }
diff --git a/internal/api/openapi.yaml b/internal/api/openapi.yaml
index d3611e134..0efadfcf9 100644
--- a/internal/api/openapi.yaml
+++ b/internal/api/openapi.yaml
@@ -1480,7 +1480,9 @@ paths:
         one-hour safety margin, and the body must echo the quote from GET
         /api/purchases/{id}/revoke/calculate. An already revoked row answers
         "already_revoked"; a row whose Azure refund was issued but not yet
-        recorded answers code RECONCILE_PENDING and must not be retried.
+        recorded, or is being posted by a concurrent revoke of the same
+        purchase, answers code RECONCILE_PENDING and must not be retried.
+        Only one revoke at a time posts the Azure return.
       parameters:
         - $ref: '#/components/parameters/CSRFToken'
       requestBody:
@@ -8321,7 +8323,8 @@ components:
         One of three shapes. A canceled scheduled execution: status
         "canceled" and message. A revoked purchase history row: status
         "revoked" or "already_revoked" with revoked_at and revoked_via. An
-        Azure refund that was issued but could not be recorded yet: code
+        Azure refund that was issued but could not be recorded yet, or that a
+        concurrent revoke of the same purchase is posting: code
         RECONCILE_PENDING, azure_returned and message (do not retry).
       properties:
         status:
diff --git a/internal/config/errors.go b/internal/config/errors.go
index 691f86bbc..32dcb0901 100644
--- a/internal/config/errors.go
+++ b/internal/config/errors.go
@@ -18,6 +18,13 @@ var ErrEmptyPurchaseID = errors.New("purchase history record has no purchase id"
 // start another provider return (issue #316).
 var ErrPurchaseAlreadyRevoked = errors.New("purchase is already revoked")
 
+// ErrRevocationInFlight is returned (wrapped) by
+// FlipPurchaseRevocationInFlight when the purchase_history row is unrevoked
+// but another request already holds the in-flight claim: that request is
+// posting (or has posted) the provider return, so the caller must not post
+// another one (issue #1518).
+var ErrRevocationInFlight = errors.New("purchase revocation is already in flight")
+
 // ErrPurchasePlanConflict means a plan changed after the caller read it.
 var ErrPurchasePlanConflict = errors.New("purchase plan changed")
 
diff --git a/internal/config/interfaces.go b/internal/config/interfaces.go
index 4f917b0d3..9cc22be24 100644
--- a/internal/config/interfaces.go
+++ b/internal/config/interfaces.go
@@ -410,14 +410,16 @@ type StoreInterface interface {
 	// it is already non-null). Used by the revoke endpoint (issue #290).
 	MarkPurchaseRevoked(ctx context.Context, purchaseID string, revokedAt time.Time, revokedVia string, supportCaseID string, calcRefundAmount *float64, calcRefundCurrency string) error
 
-	// FlipPurchaseRevocationInFlight atomically sets revocation_in_flight=true
-	// on a purchase_history row. Called immediately before the Azure Return API
-	// call so that the row can be identified by the finalize sweep if the
-	// subsequent MarkPurchaseRevoked DB write fails (partial-success reconciliation,
-	// issue #290 Finding #6, migration 000072). No-op when the flag is already
-	// true (idempotent). Only an unrevoked row is flipped: a revoked one
-	// returns an error wrapping ErrPurchaseAlreadyRevoked (issue #316).
-	// Returns a not-found error when no row matches.
+	// FlipPurchaseRevocationInFlight atomically claims a purchase_history row
+	// by setting revocation_in_flight=true. Called immediately before the Azure
+	// Return API call so that the row can be identified by the finalize sweep if
+	// the subsequent MarkPurchaseRevoked DB write fails (partial-success
+	// reconciliation, issue #290 Finding #6, migration 000072). Only an
+	// unrevoked, unflagged row is flipped, so exactly one concurrent caller
+	// wins: a revoked row returns an error wrapping ErrPurchaseAlreadyRevoked
+	// (issue #316), and a row another request already flagged returns an error
+	// wrapping ErrRevocationInFlight (issue #1518). Returns a not-found error
+	// when no row matches.
 	FlipPurchaseRevocationInFlight(ctx context.Context, purchaseID string) error
 
 	// ClearRevocationInFlight resets revocation_in_flight=false on a
@@ -426,7 +428,9 @@ type StoreInterface interface {
 	// left in a permanently-sticky in-flight state that would prevent future
 	// retries or mislead the finalize_revocations sweep (issue #290, second-wave
 	// CR Finding D). No-op when the row is already false. Best-effort: callers
-	// should log on error but not surface it to the user.
+	// should log on error but not surface it to the user. Only the caller whose
+	// FlipPurchaseRevocationInFlight succeeded may call it, so it never clears
+	// a flag another request set (issue #1518).
 	ClearRevocationInFlight(ctx context.Context, purchaseID string) error
 
 	// GetPurchaseHistoryInFlight returns all purchase_history rows with
diff --git a/internal/config/store_postgres.go b/internal/config/store_postgres.go
index 7ddb10820..3f4117b11 100644
--- a/internal/config/store_postgres.go
+++ b/internal/config/store_postgres.go
@@ -2971,27 +2971,32 @@ func (s *PostgresStore) MarkPurchaseRevoked(ctx context.Context, purchaseID stri
 	return nil
 }
 
-// FlipPurchaseRevocationInFlight sets revocation_in_flight=true on the
-// purchase_history row for purchaseID. Called immediately before the Azure
-// Return API call to enable partial-success reconciliation (issue #290
-// Finding #6). Idempotent: an unrevoked row whose flag is already true is
-// left as it is.
+// FlipPurchaseRevocationInFlight claims the revocation of the
+// purchase_history row for purchaseID by setting revocation_in_flight=true.
+// Called immediately before the Azure Return API call to enable
+// partial-success reconciliation (issue #290 Finding #6).
 //
-// Compare-and-set on revoked_at IS NULL (issue #316): only an unrevoked row
-// can be flipped. A stale concurrent revoke request that reaches the flip
-// after another request's MarkPurchaseRevoked committed would otherwise set
-// the flag again on a revoked row, which MarkPurchaseRevoked (guarded on the
-// same predicate) then never clears. The row lock serializes the two
-// UPDATEs, and the flip re-checks the predicate on the committed row, so the
-// flag ends false whichever lands first. A revoked row returns an error
-// wrapping ErrPurchaseAlreadyRevoked; a missing row keeps the not-found
-// error.
+// It is a compare-and-set on two predicates:
+//   - revoked_at IS NULL (issue #316): a stale request that reaches the flip
+//     after another request's MarkPurchaseRevoked committed must not flag the
+//     revoked row again, since MarkPurchaseRevoked (guarded on the same
+//     predicate) would then never clear it. That returns an error wrapping
+//     ErrPurchaseAlreadyRevoked.
+//   - revocation_in_flight = false (issue #1518): only one request may hold
+//     the claim, so two first-time revokes cannot both post a Return. The
+//     request that loses gets an error wrapping ErrRevocationInFlight.
+//
+// The row lock serializes concurrent flips and marks, and under READ
+// COMMITTED a waiting UPDATE re-checks both predicates against the committed
+// row, so exactly one flip of an unrevoked, unflagged row succeeds. A missing
+// row keeps the not-found error.
 func (s *PostgresStore) FlipPurchaseRevocationInFlight(ctx context.Context, purchaseID string) error {
 	tag, err := s.db.Exec(ctx, `
 		UPDATE purchase_history
 		   SET revocation_in_flight = true
 		 WHERE purchase_id = $1
 		   AND revoked_at IS NULL
+		   AND revocation_in_flight = false
 	`, purchaseID)
 	if err != nil {
 		return fmt.Errorf("FlipPurchaseRevocationInFlight: %w", err)
@@ -2999,21 +3004,35 @@ func (s *PostgresStore) FlipPurchaseRevocationInFlight(ctx context.Context, purc
 	if tag.RowsAffected() > 0 {
 		return nil
 	}
-	var exists bool
-	if err := s.db.QueryRow(ctx, `SELECT EXISTS(SELECT 1 FROM purchase_history WHERE purchase_id = $1)`, purchaseID).Scan(&exists); err != nil {
+	// Nothing matched: tell a missing row from a revoked one and from one
+	// another request has claimed. A row that is unrevoked when re-read lost
+	// the claim to a concurrent request; it is reported in flight even if
+	// that request has since cleared the flag, so this caller never posts a
+	// Return it did not claim.
+	var total, unrevoked int
+	if err := s.db.QueryRow(ctx, `
+		SELECT count(*), count(*) FILTER (WHERE revoked_at IS NULL)
+		  FROM purchase_history
+		 WHERE purchase_id = $1
+	`, purchaseID).Scan(&total, &unrevoked); err != nil {
 		return fmt.Errorf("FlipPurchaseRevocationInFlight existence check: %w", err)
 	}
-	if !exists {
+	if total == 0 {
 		return fmt.Errorf("FlipPurchaseRevocationInFlight: purchase_id %q not found", purchaseID)
 	}
-	return fmt.Errorf("FlipPurchaseRevocationInFlight: purchase_id %q: %w", purchaseID, ErrPurchaseAlreadyRevoked)
+	if unrevoked == 0 {
+		return fmt.Errorf("FlipPurchaseRevocationInFlight: purchase_id %q: %w", purchaseID, ErrPurchaseAlreadyRevoked)
+	}
+	return fmt.Errorf("FlipPurchaseRevocationInFlight: purchase_id %q: %w", purchaseID, ErrRevocationInFlight)
 }
 
 // ClearRevocationInFlight sets revocation_in_flight=false on a purchase_history
 // row. Called when the Azure Return call fails transiently (before Azure actually
 // issued a refund) so the row is not left stuck in the in-flight state, which
 // would mislead the finalize_revocations sweep into thinking Azure succeeded
-// (issue #290, second-wave CR Finding D). No-op when already false.
+// (issue #290, second-wave CR Finding D). No-op when already false. The
+// UPDATE is unconditional: only the request whose
+// FlipPurchaseRevocationInFlight claimed the row may call it (issue #1518).
 func (s *PostgresStore) ClearRevocationInFlight(ctx context.Context, purchaseID string) error {
 	tag, err := s.db.Exec(ctx, `
 		UPDATE purchase_history
diff --git a/internal/config/store_postgres_issue316_integration_test.go b/internal/config/store_postgres_issue316_integration_test.go
index 0a84b1abb..2e072535f 100644
--- a/internal/config/store_postgres_issue316_integration_test.go
+++ b/internal/config/store_postgres_issue316_integration_test.go
@@ -6,6 +6,7 @@ package config
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"sync"
 	"testing"
@@ -148,11 +149,14 @@ func TestFlipPurchaseRevocationInFlight_CompareAndSet(t *testing.T) {
 		assert.Zero(t, inFlightIndexed(ctx, t, store, id))
 	})
 
-	t.Run("an unrevoked row still flips, idempotently", func(t *testing.T) {
+	t.Run("an unrevoked row flips once; a second flip finds it in flight", func(t *testing.T) {
 		const id = "azure-flip-316-b"
 		seedRevocationRow(ctx, t, store, id)
 		require.NoError(t, store.FlipPurchaseRevocationInFlight(ctx, id))
-		require.NoError(t, store.FlipPurchaseRevocationInFlight(ctx, id), "a second flip of an in-flight row is a no-op")
+		err := store.FlipPurchaseRevocationInFlight(ctx, id)
+		require.ErrorIs(t, err, ErrRevocationInFlight, "a second flip of an in-flight row must not claim it again (issue #1518)")
+		assert.NotErrorIs(t, err, ErrPurchaseAlreadyRevoked)
+		assert.NotContains(t, err.Error(), "not found")
 		assert.Equal(t, 1, inFlightIndexed(ctx, t, store, id))
 	})
 
@@ -160,6 +164,7 @@ func TestFlipPurchaseRevocationInFlight_CompareAndSet(t *testing.T) {
 		err := store.FlipPurchaseRevocationInFlight(ctx, "azure-flip-316-missing")
 		require.ErrorContains(t, err, "not found")
 		assert.NotErrorIs(t, err, ErrPurchaseAlreadyRevoked)
+		assert.NotErrorIs(t, err, ErrRevocationInFlight)
 	})
 
 	// The race the guard closes: one request's MarkPurchaseRevoked against
@@ -167,7 +172,8 @@ func TestFlipPurchaseRevocationInFlight_CompareAndSet(t *testing.T) {
 	// serializes them; a flip that waits on the mark re-checks
 	// revoked_at IS NULL on the committed row and matches nothing. Whatever
 	// the interleaving, the row ends revoked with the flag clear, and every
-	// flip either landed before the mark or reports the row revoked.
+	// flip reports the row either still in flight (it ran before the mark;
+	// the row was already claimed, issue #1518) or revoked.
 	t.Run("stale flips racing a mark", func(t *testing.T) {
 		const rounds, flips = 20, 4
 		for round := range rounds {
@@ -195,7 +201,8 @@ func TestFlipPurchaseRevocationInFlight_CompareAndSet(t *testing.T) {
 
 			require.NoError(t, errs[0], "round %d: the mark", round)
 			for i, err := range errs[1:] {
-				if err != nil {
+				require.Error(t, err, "round %d: flip %d must not claim a row that is already in flight", round, i)
+				if !errors.Is(err, ErrRevocationInFlight) {
 					require.ErrorIs(t, err, ErrPurchaseAlreadyRevoked, "round %d: flip %d", round, i)
 				}
 			}
@@ -207,6 +214,61 @@ func TestFlipPurchaseRevocationInFlight_CompareAndSet(t *testing.T) {
 	})
 }
 
+// TestFlipPurchaseRevocationInFlight_ExclusiveClaim covers issue #1518: the
+// flip let a row that was already in flight be flipped again, so two
+// first-time revokes could both pass it and both post an Azure Return. Now
+// it is a claim: of any number of concurrent flips of one unrevoked,
+// unflagged row exactly one succeeds and the rest get ErrRevocationInFlight.
+// After the holder clears the flag (its Return failed) the row can be
+// claimed again.
+func TestFlipPurchaseRevocationInFlight_ExclusiveClaim(t *testing.T) {
+	ctx := context.Background()
+	store := setupRampStepStore(ctx, t)
+
+	t.Run("concurrent first flips: exactly one claims the row", func(t *testing.T) {
+		const rounds, flips = 20, 6
+		for round := range rounds {
+			id := fmt.Sprintf("azure-flip-1518-race-%d", round)
+			seedRevocationRow(ctx, t, store, id)
+
+			errs := make([]error, flips)
+			start := make(chan struct{})
+			var wg sync.WaitGroup
+			for i := range flips {
+				wg.Add(1)
+				go func() {
+					defer wg.Done()
+					<-start
+					errs[i] = store.FlipPurchaseRevocationInFlight(ctx, id)
+				}()
+			}
+			close(start)
+			wg.Wait()
+
+			winners := 0
+			for i, err := range errs {
+				if err == nil {
+					winners++
+					continue
+				}
+				require.ErrorIs(t, err, ErrRevocationInFlight, "round %d: flip %d", round, i)
+			}
+			require.Equal(t, 1, winners, "round %d: exactly one concurrent flip may claim the row", round)
+			require.Equal(t, 1, inFlightIndexed(ctx, t, store, id), "round %d", round)
+		}
+	})
+
+	t.Run("the holder's clear releases the claim", func(t *testing.T) {
+		const id = "azure-flip-1518-release"
+		seedRevocationRow(ctx, t, store, id)
+		require.NoError(t, store.FlipPurchaseRevocationInFlight(ctx, id))
+		require.ErrorIs(t, store.FlipPurchaseRevocationInFlight(ctx, id), ErrRevocationInFlight)
+		require.NoError(t, store.ClearRevocationInFlight(ctx, id))
+		require.NoError(t, store.FlipPurchaseRevocationInFlight(ctx, id), "a cleared row can be claimed by a later revoke")
+		assert.Equal(t, 1, inFlightIndexed(ctx, t, store, id))
+	})
+}
+
 // TestSetPlanAccounts_ProviderCheckFailsClosed covers the store-side guard of
 // A04-012. DerivePlanProviders skipped "provider:service" keys (the form
 // migration 000040 wrote) and legacy keys with no provider prefix, and an