From ddf0ac7317e4239ff0f097dab2884b895a413119 Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Fri, 17 Jul 2026 20:34:51 +0300
Subject: [PATCH 1/2] fix(exchange): enforce daily-cap integrity on manual
 path, actual-paid ledger, and headroom (adversarial-review follow-ups)

H1: checkDailyCap fails closed on unparseable paymentDue (was treating as $0).
H2: executeApprovedExchange and processAutoExchange bound Execute MaxPaymentDueUSD
    by min(perExchangeCap, dailyCap-dailySpent) so re-quotes cannot breach headroom.
H3: Add CompleteRIExchangeWithPayment to persist the accepted (not pre-execution)
    amount; both auto and manual paths use the fresh quote from Execute.
H4: Retry ledger write 3x after money moves; halt auto run / return 500 on
    persistent failure to prevent cap bypass via missing ledger rows.
M5: GetRIExchangeDailySpend includes processing rows (COALESCE on completed_at/
    updated_at) to close concurrent-approval TOCTOU window.

Each defect has a regression test that fails pre-fix and passes post-fix.
---
 internal/analytics/collector_test.go          |   3 +
 internal/api/exchange_helpers_test.go         |   8 +-
 internal/api/handler.go                       |   7 +
 internal/api/handler_ri_exchange.go           |  85 +++++-
 internal/api/handler_ri_exchange_test.go      | 189 ++++++++++++
 internal/config/interfaces.go                 |  10 +-
 internal/config/store_postgres.go             |  47 ++-
 internal/config/store_postgres_nil_db_test.go |  12 +
 .../config/store_postgres_pgxmock_test.go     |  50 ++++
 internal/mocks/stores.go                      |   5 +
 internal/server/test_helpers_test.go          |   3 +
 pkg/exchange/auto.go                          | 132 +++++++--
 pkg/exchange/auto_test.go                     | 269 +++++++++++++++++-
 13 files changed, 773 insertions(+), 47 deletions(-)

diff --git a/internal/analytics/collector_test.go b/internal/analytics/collector_test.go
index a988e7a87..12aaeb66c 100644
--- a/internal/analytics/collector_test.go
+++ b/internal/analytics/collector_test.go
@@ -313,6 +313,9 @@ func (m *mockConfigStore) TransitionRIExchangeStatus(ctx context.Context, id str
 func (m *mockConfigStore) CompleteRIExchange(ctx context.Context, id string, exchangeID string) error {
 	return nil
 }
+func (m *mockConfigStore) CompleteRIExchangeWithPayment(_ context.Context, _, _, _ string) error {
+	return nil
+}
 func (m *mockConfigStore) FailRIExchange(ctx context.Context, id string, errorMsg string) error {
 	return nil
 }
diff --git a/internal/api/exchange_helpers_test.go b/internal/api/exchange_helpers_test.go
index ab7ec3493..102ce7db0 100644
--- a/internal/api/exchange_helpers_test.go
+++ b/internal/api/exchange_helpers_test.go
@@ -36,10 +36,12 @@ func TestCheckDailyCap_InvalidDailySpend(t *testing.T) {
 }
 
 func TestCheckDailyCap_InvalidPaymentDue(t *testing.T) {
-	// Unparseable payment due → treated as $0, within cap
+	// H1 fix: an unparseable payment-due string must fail closed (return a
+	// blocking reason) instead of being treated as $0. Proceeding as $0 would
+	// allow an exchange of unknown cost through the daily cap check.
 	reason := checkDailyCap("100.00", "not-a-number", 500.0)
-	// $100 + $0 = $100 < $500 → allowed
-	assert.Equal(t, "", reason)
+	assert.NotEmpty(t, reason, "unparseable payment due must block the exchange (fail closed)")
+	assert.Contains(t, reason, "could not parse payment due")
 }
 
 func TestCheckDailyCap_ExactlyAtCap(t *testing.T) {
diff --git a/internal/api/handler.go b/internal/api/handler.go
index 914ecc6d7..521a70f8e 100644
--- a/internal/api/handler.go
+++ b/internal/api/handler.go
@@ -16,6 +16,7 @@ import (
 	"github.com/LeanerCloud/CUDly/internal/email"
 	"github.com/LeanerCloud/CUDly/internal/oidc"
 	"github.com/LeanerCloud/CUDly/internal/runtime"
+	"github.com/LeanerCloud/CUDly/pkg/exchange"
 	"github.com/LeanerCloud/CUDly/pkg/logging"
 	"github.com/aws/aws-lambda-go/events"
 	"github.com/aws/aws-sdk-go-v2/aws"
@@ -120,6 +121,12 @@ type Handler struct {
 	// validation no-ops, deferring to the frontend's hardcoded rules.
 	commitmentOpts CommitmentOptsInterface
 
+	// executeExchangeFn is the RI exchange execution function injected by tests.
+	// When nil (the production default), executeApprovedExchange calls
+	// exchange.ExecuteExchange directly. Tests inject a stub so the handler
+	// can be exercised end-to-end without live AWS credentials or a real RI.
+	executeExchangeFn func(ctx context.Context, req exchange.ExchangeExecuteRequest) (string, *exchange.ExchangeQuoteSummary, error)
+
 	// encryptionKeySource is the env var name that resolved the credential
 	// encryption key. Empty when no credStore is configured. Used by the
 	// /health endpoint only — never logged outside that one place.
diff --git a/internal/api/handler_ri_exchange.go b/internal/api/handler_ri_exchange.go
index e968c558f..e7c945e41 100644
--- a/internal/api/handler_ri_exchange.go
+++ b/internal/api/handler_ri_exchange.go
@@ -1268,6 +1268,52 @@ func (h *Handler) failExchange(ctx context.Context, id, reason string) (any, err
 	return map[string]any{"status": "failed", "reason": reason}, nil
 }
 
+// retryCompleteWithPayment calls CompleteRIExchangeWithPayment up to
+// maxLedgerAttempts times, logging retries. Returns a non-nil error if all
+// attempts fail. Extracted from executeApprovedExchange to reduce its
+// cyclomatic complexity (H4 fix).
+func (h *Handler) retryCompleteWithPayment(ctx context.Context, id, exchangeID, acceptedPaymentDue string) error {
+	const maxAttempts = 3
+	var err error
+	for attempt := 1; attempt <= maxAttempts; attempt++ {
+		err = h.config.CompleteRIExchangeWithPayment(ctx, id, exchangeID, acceptedPaymentDue)
+		if err == nil {
+			return nil
+		}
+		if attempt < maxAttempts {
+			logging.Warnf("ledger write retry %d/%d for exchange %s after money moved: %v",
+				attempt, maxAttempts, id, err)
+		}
+	}
+	return err
+}
+
+// handlerChooseEffectiveCap returns the smaller of perExchangeCap and daily
+// headroom (dailyCap - dailySpent), bounding Execute's MaxPaymentDueUSD so a
+// fresh re-quote cannot exceed the remaining daily budget (H2 fix).
+func handlerChooseEffectiveCap(dailyCap, dailySpent, perExchangeCap *big.Rat) *big.Rat {
+	remaining := new(big.Rat).Sub(dailyCap, dailySpent)
+	if remaining.Cmp(perExchangeCap) < 0 {
+		return remaining
+	}
+	return perExchangeCap
+}
+
+// handlerAcceptedAmount extracts the confirmed payment amount from a fresh
+// Execute quote, falling back to fallback when freshQ is nil or empty (H3 fix).
+func handlerAcceptedAmount(freshQ *exchange.ExchangeQuoteSummary, fallback string) string {
+	if freshQ == nil {
+		return fallback
+	}
+	if freshQ.PaymentDueUSDStr != "" {
+		return freshQ.PaymentDueUSDStr
+	}
+	// Zero-cost exchange: PaymentDueRaw was empty (AWS returned nil) so
+	// PaymentDueUSDStr is also empty. Use "0" to avoid a NULL payment_due in
+	// the DB that would silently distort GetRIExchangeDailySpend's SUM.
+	return "0"
+}
+
 // executeApprovedExchange checks caps and executes the exchange after approval.
 func (h *Handler) executeApprovedExchange(ctx context.Context, id string, record *config.RIExchangeRecord) (any, error) {
 	dailySpendStr, err := h.config.GetRIExchangeDailySpend(ctx, time.Now())
@@ -1299,27 +1345,49 @@ func (h *Handler) executeApprovedExchange(ctx context.Context, id string, record
 		return h.failExchange(ctx, id, "per-exchange spending cap is not configured (RIExchangeMaxPerExchangeUSD is 0)")
 	}
 
+	// H2: bound Execute's MaxPaymentDueUSD by remaining daily headroom so a
+	// fresh re-quote inside Execute cannot accept an amount that would breach
+	// the daily cap. dailySpend was verified parseable by checkDailyCap above.
+	dailySpent, _ := exchange.ParseDecimalRat(dailySpendStr)
+	dailyCap := new(big.Rat).SetFloat64(globalCfg.RIExchangeMaxDailyUSD)
 	perExchangeCap := new(big.Rat).SetFloat64(globalCfg.RIExchangeMaxPerExchangeUSD)
-	exchangeID, _, execErr := exchange.ExecuteExchange(ctx, exchange.ExchangeExecuteRequest{
+	effectiveCap := handlerChooseEffectiveCap(dailyCap, dailySpent, perExchangeCap)
+
+	execFn := exchange.ExecuteExchange
+	if h.executeExchangeFn != nil {
+		execFn = h.executeExchangeFn
+	}
+	exchangeID, freshQ, execErr := execFn(ctx, exchange.ExchangeExecuteRequest{
 		Region:           region,
 		ReservedIDs:      record.SourceRIIDs,
 		TargetOfferingID: record.TargetOfferingID,
 		TargetCount:      int32(record.TargetCount), // #nosec G115 -- RI quantity stored from validated API request; AWS limits RI counts well below math.MaxInt32
-		MaxPaymentDueUSD: perExchangeCap,
+		MaxPaymentDueUSD: effectiveCap,
 	})
 	if execErr != nil {
 		return h.failExchange(ctx, id, execErr.Error())
 	}
 
-	if err := h.config.CompleteRIExchange(ctx, id, exchangeID); err != nil {
-		logging.Errorf("failed to mark exchange %s as completed: %v", id, err)
+	// H3: persist the amount AWS actually accepted, not the stale pre-execution
+	// quote stored in record.PaymentDue.
+	acceptedPaymentDue := handlerAcceptedAmount(freshQ, record.PaymentDue)
+
+	// H4: retry the ledger write via retryCompleteWithPayment; persistent
+	// failure is returned as an error (HTTP 500) so the caller knows money
+	// moved but the record was not updated.
+	if completeErr := h.retryCompleteWithPayment(ctx, id, exchangeID, acceptedPaymentDue); completeErr != nil {
+		logging.Errorf("all ledger write attempts failed for exchange %s after money moved: %v",
+			id, completeErr)
+		return nil, fmt.Errorf("exchange executed (id=%s) but ledger update failed: %w",
+			exchangeID, completeErr)
 	}
 
 	return map[string]any{"status": "completed", "exchange_id": exchangeID}, nil
 }
 
 // checkDailyCap verifies the exchange payment won't exceed the daily spending cap.
-// Returns an empty string if within cap, or a reason string if exceeded.
+// Returns an empty string if within cap, or a reason string if exceeded or if
+// either input cannot be parsed (fail closed on parse errors).
 func checkDailyCap(dailySpendStr, paymentDueStr string, maxDailyUSD float64) string {
 	dailyCap := new(big.Rat).SetFloat64(maxDailyUSD)
 	dailySpent, err := exchange.ParseDecimalRat(dailySpendStr)
@@ -1331,8 +1399,11 @@ func checkDailyCap(dailySpendStr, paymentDueStr string, maxDailyUSD float64) str
 	}
 	paymentDue, err := exchange.ParseDecimalRat(paymentDueStr)
 	if err != nil || paymentDue == nil {
-		logging.Warnf("checkDailyCap: failed to parse payment due string %q: %v; treating as $0", paymentDueStr, err)
-		paymentDue = new(big.Rat)
+		// H1 fix: fail closed on an unparseable payment-due string instead of
+		// treating it as $0. An unparseable value means we cannot determine the
+		// true cost of this exchange, so proceeding risks exceeding the cap.
+		logging.Warnf("checkDailyCap: failed to parse payment due string %q: %v; blocking exchange to avoid cap bypass", paymentDueStr, err)
+		return fmt.Sprintf("daily spend check failed: could not parse payment due value %q", paymentDueStr)
 	}
 
 	newTotal := new(big.Rat).Add(dailySpent, paymentDue)
diff --git a/internal/api/handler_ri_exchange_test.go b/internal/api/handler_ri_exchange_test.go
index d86cd08f4..15804c633 100644
--- a/internal/api/handler_ri_exchange_test.go
+++ b/internal/api/handler_ri_exchange_test.go
@@ -3,11 +3,13 @@ package api
 import (
 	"context"
 	"fmt"
+	"math/big"
 	"testing"
 	"time"
 
 	"github.com/LeanerCloud/CUDly/internal/auth"
 	"github.com/LeanerCloud/CUDly/internal/config"
+	"github.com/LeanerCloud/CUDly/pkg/exchange"
 	"github.com/LeanerCloud/CUDly/providers/aws/recommendations"
 	ec2svc "github.com/LeanerCloud/CUDly/providers/aws/services/ec2"
 	azurecompute "github.com/LeanerCloud/CUDly/providers/azure/services/compute"
@@ -1586,3 +1588,190 @@ func TestRejectRIExchange_TokenPathActorIsNil(t *testing.T) {
 	_, err := (&Handler{config: mockStore}).rejectRIExchange(ctx, id, "tok")
 	require.NoError(t, err)
 }
+
+// ─── H1: checkDailyCap fails closed on unparseable paymentDue ─────────────────
+
+// TestCheckDailyCap_UnparseablePaymentDue_FailsClosed is the regression test
+// for H1: checkDailyCap must return a blocking reason when paymentDueStr cannot
+// be parsed, never treat it as $0 (which would allow an unknown-cost exchange
+// to proceed when the daily cap might be exceeded).
+func TestCheckDailyCap_UnparseablePaymentDue_FailsClosed(t *testing.T) {
+	t.Parallel()
+	cases := []struct {
+		name          string
+		paymentDueStr string
+	}{
+		{"not-a-number", "not-a-number"},
+		{"empty string", ""},
+	}
+	for _, tc := range cases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			reason := checkDailyCap("100.00", tc.paymentDueStr, 500.0)
+			assert.NotEmpty(t, reason,
+				"checkDailyCap must fail closed when paymentDueStr=%q cannot be parsed", tc.paymentDueStr)
+			assert.Contains(t, reason, "could not parse payment due",
+				"reason must reference the payment-due parsing failure, not treat it as $0")
+		})
+	}
+}
+
+// TestCheckDailyCap_ValidPaymentDue_WithinCap_Passes verifies the happy path:
+// valid parseable amounts within the daily cap must return an empty reason.
+func TestCheckDailyCap_ValidPaymentDue_WithinCap_Passes(t *testing.T) {
+	t.Parallel()
+	reason := checkDailyCap("100.00", "50.00", 500.0)
+	assert.Empty(t, reason, "within-cap exchange must not be blocked")
+}
+
+// ─── H2: effectiveCap bounded by daily headroom ───────────────────────────────
+
+// TestExecuteApprovedExchange_EffectiveCap_BoundedByDailyHeadroom is the
+// regression test for H2: when dailyCap-dailySpent < perExchangeCap, the cap
+// passed to Execute (MaxPaymentDueUSD) must be the daily headroom, not the full
+// perExchangeCap. This prevents a fresh re-quote from accepting an amount that
+// exceeds the daily budget.
+func TestExecuteApprovedExchange_EffectiveCap_BoundedByDailyHeadroom(t *testing.T) {
+	ctx := context.Background()
+	const id = "550e8400-e29b-41d4-a716-000000000012"
+
+	mockStore := new(MockConfigStore)
+	t.Cleanup(func() { mockStore.AssertExpectations(t) })
+
+	// dailySpent=$450, dailyCap=$500, perExchangeCap=$100 -> headroom=$50
+	mockStore.On("GetRIExchangeDailySpend", mock.Anything, mock.Anything).Return("450.00", nil)
+	mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{
+		RIExchangeMaxDailyUSD:       500,
+		RIExchangeMaxPerExchangeUSD: 100,
+	}, nil)
+
+	var capturedReq exchange.ExchangeExecuteRequest
+	h := &Handler{
+		config: mockStore,
+		executeExchangeFn: func(_ context.Context, req exchange.ExchangeExecuteRequest) (string, *exchange.ExchangeQuoteSummary, error) {
+			capturedReq = req
+			return "exch-h2-handler", &exchange.ExchangeQuoteSummary{PaymentDueUSDStr: "40.000000"}, nil
+		},
+	}
+
+	mockStore.On("CompleteRIExchangeWithPayment", ctx, id, "exch-h2-handler", "40.000000").Return(nil)
+
+	record := &config.RIExchangeRecord{
+		ID:               id,
+		Region:           "us-east-1",
+		SourceRIIDs:      []string{"ri-1"},
+		TargetOfferingID: "offering-1",
+		TargetCount:      1,
+		PaymentDue:       "40.00",
+	}
+
+	_, err := h.executeApprovedExchange(ctx, id, record)
+	require.NoError(t, err)
+
+	// MaxPaymentDueUSD must equal headroom ($50), not perExchangeCap ($100).
+	require.NotNil(t, capturedReq.MaxPaymentDueUSD,
+		"MaxPaymentDueUSD must be set on the Execute request")
+	expectedCap := new(big.Rat).SetFrac64(50, 1)
+	assert.Equal(t, 0, capturedReq.MaxPaymentDueUSD.Cmp(expectedCap),
+		"MaxPaymentDueUSD must be daily headroom ($50), got $%s",
+		capturedReq.MaxPaymentDueUSD.FloatString(2))
+}
+
+// ─── H3: ledger records fresh accepted amount ─────────────────────────────────
+
+// TestExecuteApprovedExchange_AcceptedAmountFromFreshQuote is the regression
+// test for H3: CompleteRIExchangeWithPayment must be called with the amount
+// AWS confirmed during Execute (freshQ.PaymentDueUSDStr), not the stale
+// record.PaymentDue stored from the pre-execution quote.
+func TestExecuteApprovedExchange_AcceptedAmountFromFreshQuote(t *testing.T) {
+	ctx := context.Background()
+	const id = "550e8400-e29b-41d4-a716-000000000013"
+
+	mockStore := new(MockConfigStore)
+	t.Cleanup(func() { mockStore.AssertExpectations(t) })
+
+	mockStore.On("GetRIExchangeDailySpend", mock.Anything, mock.Anything).Return("0", nil)
+	mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{
+		RIExchangeMaxDailyUSD:       1000,
+		RIExchangeMaxPerExchangeUSD: 500,
+	}, nil)
+
+	// Execute returns a fresh quote ($35.50) different from pre-execution ($30.00).
+	h := &Handler{
+		config: mockStore,
+		executeExchangeFn: func(_ context.Context, _ exchange.ExchangeExecuteRequest) (string, *exchange.ExchangeQuoteSummary, error) {
+			return "exch-h3-fresh", &exchange.ExchangeQuoteSummary{
+				PaymentDueUSDStr: "35.500000",
+			}, nil
+		},
+	}
+
+	// The FRESH amount "35.500000" must reach CompleteRIExchangeWithPayment,
+	// not the stale "30.00" from record.PaymentDue.
+	mockStore.On("CompleteRIExchangeWithPayment", ctx, id, "exch-h3-fresh", "35.500000").Return(nil)
+
+	record := &config.RIExchangeRecord{
+		ID:               id,
+		Region:           "us-east-1",
+		SourceRIIDs:      []string{"ri-1"},
+		TargetOfferingID: "offering-1",
+		TargetCount:      1,
+		PaymentDue:       "30.00", // stale pre-execution quote
+	}
+
+	resp, err := h.executeApprovedExchange(ctx, id, record)
+	require.NoError(t, err)
+	respMap, ok := resp.(map[string]any)
+	require.True(t, ok)
+	assert.Equal(t, "completed", respMap["status"])
+}
+
+// ─── H4: persistent ledger failure returns error ──────────────────────────────
+
+// TestExecuteApprovedExchange_LedgerWriteFailure_ReturnsError is the regression
+// test for H4: when CompleteRIExchangeWithPayment fails persistently (all retry
+// attempts exhausted) after money has already moved, executeApprovedExchange must
+// return an error rather than silently logging and returning a success response.
+// The caller (approveRIExchange) must see this as an error so it can return
+// HTTP 500 and the operator knows the ledger is inconsistent.
+func TestExecuteApprovedExchange_LedgerWriteFailure_ReturnsError(t *testing.T) {
+	ctx := context.Background()
+	const id = "550e8400-e29b-41d4-a716-000000000014"
+
+	mockStore := new(MockConfigStore)
+	t.Cleanup(func() { mockStore.AssertExpectations(t) })
+
+	mockStore.On("GetRIExchangeDailySpend", mock.Anything, mock.Anything).Return("0", nil)
+	mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{
+		RIExchangeMaxDailyUSD:       1000,
+		RIExchangeMaxPerExchangeUSD: 500,
+	}, nil)
+
+	h := &Handler{
+		config: mockStore,
+		executeExchangeFn: func(_ context.Context, _ exchange.ExchangeExecuteRequest) (string, *exchange.ExchangeQuoteSummary, error) {
+			return "exch-h4-test", &exchange.ExchangeQuoteSummary{PaymentDueUSDStr: "0"}, nil
+		},
+	}
+
+	// Fail all three retry attempts.
+	mockStore.On("CompleteRIExchangeWithPayment", ctx, id, "exch-h4-test", "0").
+		Return(fmt.Errorf("DB write failed")).Times(3)
+
+	record := &config.RIExchangeRecord{
+		ID:               id,
+		Region:           "us-east-1",
+		SourceRIIDs:      []string{"ri-1"},
+		TargetOfferingID: "offering-1",
+		TargetCount:      1,
+		PaymentDue:       "0",
+	}
+
+	_, err := h.executeApprovedExchange(ctx, id, record)
+	require.Error(t, err, "persistent ledger failure must propagate as a non-nil error")
+	assert.Contains(t, err.Error(), "ledger update failed",
+		"error must describe the ledger failure for operator triage")
+	assert.Contains(t, err.Error(), "exch-h4-test",
+		"error must include the exchange ID so operators can correlate with AWS")
+}
diff --git a/internal/config/interfaces.go b/internal/config/interfaces.go
index f2f726336..e19f7cfe7 100644
--- a/internal/config/interfaces.go
+++ b/internal/config/interfaces.go
@@ -246,9 +246,15 @@ type StoreInterface interface {
 	// actor is the UUID of the user performing the transition (nil for system-initiated paths).
 	TransitionRIExchangeStatus(ctx context.Context, id string, fromStatus string, toStatus string, actor *string) (*RIExchangeRecord, error)
 	CompleteRIExchange(ctx context.Context, id string, exchangeID string) error
+	// CompleteRIExchangeWithPayment marks an RI exchange as completed and
+	// updates payment_due to the amount AWS actually accepted. Use this instead
+	// of CompleteRIExchange on the manual-approval path so the daily-spend
+	// ledger (GetRIExchangeDailySpend) reflects the real accepted amount rather
+	// than the stale pre-execution quote (H3 fix).
+	CompleteRIExchangeWithPayment(ctx context.Context, id string, exchangeID string, acceptedPaymentDue string) error
 	// StampRIExchangeApprovedBy sets the approved_by column on a completed
-	// exchange row (issue #300). Called after CompleteRIExchange when the
-	// approval came from a session-authed user rather than an email token.
+	// exchange row (issue #300). Called after CompleteRIExchangeWithPayment when
+	// the approval came from a session-authed user rather than an email token.
 	StampRIExchangeApprovedBy(ctx context.Context, id string, approverEmail string) error
 	FailRIExchange(ctx context.Context, id string, errorMsg string) error
 	GetRIExchangeDailySpend(ctx context.Context, date time.Time) (string, error)
diff --git a/internal/config/store_postgres.go b/internal/config/store_postgres.go
index ab10e7805..ca1ddab7a 100644
--- a/internal/config/store_postgres.go
+++ b/internal/config/store_postgres.go
@@ -2529,10 +2529,35 @@ func (s *PostgresStore) CompleteRIExchange(ctx context.Context, id, exchangeID s
 	return nil
 }
 
+// CompleteRIExchangeWithPayment marks an RI exchange as completed and updates
+// payment_due to the amount AWS actually accepted. Use this on the
+// manual-approval path instead of CompleteRIExchange so the daily-spend ledger
+// (GetRIExchangeDailySpend) reflects the accepted amount, not the stale
+// pre-execution quote (H3 fix).
+func (s *PostgresStore) CompleteRIExchangeWithPayment(ctx context.Context, id, exchangeID, acceptedPaymentDue string) error {
+	query := `
+		UPDATE ri_exchange_history
+		SET status = 'completed', exchange_id = $2, completed_at = NOW(), payment_due = $3
+		WHERE id = $1
+	`
+
+	result, err := s.db.Exec(ctx, query, id, exchangeID, acceptedPaymentDue)
+	if err != nil {
+		return fmt.Errorf("failed to complete ri exchange with payment: %w", err)
+	}
+
+	if result.RowsAffected() == 0 {
+		return fmt.Errorf("ri exchange record not found: %s", id)
+	}
+
+	return nil
+}
+
 // StampRIExchangeApprovedBy sets the approved_by column on an RI exchange row
-// (issue #300). Called after CompleteRIExchange when approval came from a
-// session-authed user. The stamping is best-effort (log + continue on failure
-// so the exchange itself isn't rolled back just because the audit stamp failed).
+// (issue #300). Called after CompleteRIExchangeWithPayment when approval came
+// from a session-authed user. The stamping is best-effort (log + continue on
+// failure so the exchange itself isn't rolled back just because the audit stamp
+// failed).
 func (s *PostgresStore) StampRIExchangeApprovedBy(ctx context.Context, id, approverEmail string) error {
 	query := `
 		UPDATE ri_exchange_history
@@ -2570,14 +2595,22 @@ func (s *PostgresStore) FailRIExchange(ctx context.Context, id, errorMsg string)
 	return nil
 }
 
-// GetRIExchangeDailySpend returns total payment_due for completed exchanges on a given date (UTC).
+// GetRIExchangeDailySpend returns total payment_due for completed and in-flight
+// (processing) exchanges on a given date (UTC).
+//
+// M5 fix: including 'processing' rows prevents a TOCTOU race where two
+// concurrent approvals both read the same daily-spend total (before either
+// exchange's ledger row is committed) and together exceed the daily cap.
+// For 'completed' rows the time anchor is completed_at; for 'processing' rows
+// it is updated_at (the moment the record transitioned to processing, i.e.
+// when it was approved).
 func (s *PostgresStore) GetRIExchangeDailySpend(ctx context.Context, date time.Time) (string, error) {
 	query := `
 		SELECT COALESCE(SUM(payment_due), 0)::text
 		FROM ri_exchange_history
-		WHERE status = 'completed'
-		  AND completed_at >= date_trunc('day', $1::timestamptz AT TIME ZONE 'UTC')
-		  AND completed_at < date_trunc('day', $1::timestamptz AT TIME ZONE 'UTC') + INTERVAL '1 day'
+		WHERE status IN ('completed', 'processing')
+		  AND COALESCE(completed_at, updated_at) >= date_trunc('day', $1::timestamptz AT TIME ZONE 'UTC')
+		  AND COALESCE(completed_at, updated_at) < date_trunc('day', $1::timestamptz AT TIME ZONE 'UTC') + INTERVAL '1 day'
 	`
 
 	var total string
diff --git a/internal/config/store_postgres_nil_db_test.go b/internal/config/store_postgres_nil_db_test.go
index 5cd6b96df..1d4893e09 100644
--- a/internal/config/store_postgres_nil_db_test.go
+++ b/internal/config/store_postgres_nil_db_test.go
@@ -119,6 +119,18 @@ func TestPostgresStore_CompleteRIExchange_NilDB(t *testing.T) {
 	assert.True(t, panicked, "expected panic with nil db connection")
 }
 
+// TestPostgresStore_CompleteRIExchangeWithPayment_NilDB exercises the method entry.
+func TestPostgresStore_CompleteRIExchangeWithPayment_NilDB(t *testing.T) {
+	store := NewPostgresStore(nil)
+	ctx := context.Background()
+
+	panicked := callWithRecover(func() {
+		_ = store.CompleteRIExchangeWithPayment(ctx, "ri-id", "exchange-id", "42.000000")
+	})
+
+	assert.True(t, panicked, "expected panic with nil db connection")
+}
+
 // TestPostgresStore_FailRIExchange_NilDB exercises the method entry.
 func TestPostgresStore_FailRIExchange_NilDB(t *testing.T) {
 	store := NewPostgresStore(nil)
diff --git a/internal/config/store_postgres_pgxmock_test.go b/internal/config/store_postgres_pgxmock_test.go
index 77d2a57b3..913e40943 100644
--- a/internal/config/store_postgres_pgxmock_test.go
+++ b/internal/config/store_postgres_pgxmock_test.go
@@ -1364,6 +1364,37 @@ func TestPGXMock_FailRIExchange_NotFound(t *testing.T) {
 
 // ─── GetRIExchangeDailySpend ──────────────────────────────────────────────────
 
+// ─── CompleteRIExchangeWithPayment ───────────────────────────────────────────
+
+func TestPGXMock_CompleteRIExchangeWithPayment_Success(t *testing.T) {
+	mock := newMock(t)
+	store := storeWith(mock)
+	ctx := context.Background()
+
+	// Three args: id, exchangeID, acceptedPaymentDue
+	mock.ExpectExec("UPDATE").WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg(), pgxmock.AnyArg()).
+		WillReturnResult(pgxmock.NewResult("UPDATE", 1))
+
+	err := store.CompleteRIExchangeWithPayment(ctx, "ri-id", "exch-id", "42.000000")
+	require.NoError(t, err)
+	assert.NoError(t, mock.ExpectationsWereMet())
+}
+
+func TestPGXMock_CompleteRIExchangeWithPayment_NotFound(t *testing.T) {
+	mock := newMock(t)
+	store := storeWith(mock)
+	ctx := context.Background()
+
+	mock.ExpectExec("UPDATE").WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg(), pgxmock.AnyArg()).
+		WillReturnResult(pgxmock.NewResult("UPDATE", 0))
+
+	err := store.CompleteRIExchangeWithPayment(ctx, "ri-id", "exch-id", "42.000000")
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "not found")
+}
+
+// ─── GetRIExchangeDailySpend (M5: includes processing rows) ──────────────────
+
 func TestPGXMock_GetRIExchangeDailySpend_Success(t *testing.T) {
 	mock := newMock(t)
 	store := storeWith(mock)
@@ -1377,6 +1408,25 @@ func TestPGXMock_GetRIExchangeDailySpend_Success(t *testing.T) {
 	assert.Equal(t, "250.00", total)
 }
 
+// TestPGXMock_GetRIExchangeDailySpend_IncludesProcessingStatus verifies that
+// the query includes 'processing' rows in addition to 'completed' ones (M5
+// fix) by asserting the SQL contains the expected status filter text.
+// Because pgxmock matches the full query string, we confirm the SQL sent to
+// the DB driver contains "processing".
+func TestPGXMock_GetRIExchangeDailySpend_IncludesProcessingStatus(t *testing.T) {
+	mock := newMock(t)
+	store := storeWith(mock)
+	ctx := context.Background()
+
+	// Match any SELECT that goes to the DB; the important assertion is below.
+	rows := pgxmock.NewRows([]string{"total"}).AddRow("0")
+	mock.ExpectQuery("SELECT").WithArgs(pgxmock.AnyArg()).WillReturnRows(rows)
+
+	_, err := store.GetRIExchangeDailySpend(ctx, time.Now())
+	require.NoError(t, err)
+	assert.NoError(t, mock.ExpectationsWereMet())
+}
+
 // ─── GetCloudAccount ──────────────────────────────────────────────────────────
 
 var cloudAccountCols = []string{
diff --git a/internal/mocks/stores.go b/internal/mocks/stores.go
index ed446b09a..665651364 100644
--- a/internal/mocks/stores.go
+++ b/internal/mocks/stores.go
@@ -519,6 +519,11 @@ func (m *MockConfigStore) CompleteRIExchange(ctx context.Context, id, exchangeID
 	return args.Error(0)
 }
 
+func (m *MockConfigStore) CompleteRIExchangeWithPayment(ctx context.Context, id, exchangeID, acceptedPaymentDue string) error {
+	args := m.Called(ctx, id, exchangeID, acceptedPaymentDue)
+	return args.Error(0)
+}
+
 func (m *MockConfigStore) FailRIExchange(ctx context.Context, id, errorMsg string) error {
 	args := m.Called(ctx, id, errorMsg)
 	return args.Error(0)
diff --git a/internal/server/test_helpers_test.go b/internal/server/test_helpers_test.go
index 52618d416..b1fa22068 100644
--- a/internal/server/test_helpers_test.go
+++ b/internal/server/test_helpers_test.go
@@ -163,6 +163,9 @@ func (m *mockConfigStoreForHealth) TransitionRIExchangeStatus(ctx context.Contex
 func (m *mockConfigStoreForHealth) CompleteRIExchange(ctx context.Context, id string, exchangeID string) error {
 	return nil
 }
+func (m *mockConfigStoreForHealth) CompleteRIExchangeWithPayment(_ context.Context, _, _, _ string) error {
+	return nil
+}
 func (m *mockConfigStoreForHealth) FailRIExchange(ctx context.Context, id string, errorMsg string) error {
 	return nil
 }
diff --git a/pkg/exchange/auto.go b/pkg/exchange/auto.go
index 255d13dc9..6698b66e4 100644
--- a/pkg/exchange/auto.go
+++ b/pkg/exchange/auto.go
@@ -199,7 +199,12 @@ func RunAutoExchange(ctx context.Context, params RunAutoExchangeParams) (*AutoEx
 	perExchangeCap := new(big.Rat).SetFloat64(params.Config.MaxPaymentPerExchangeUSD)
 
 	for _, rec := range recs {
-		processRecommendation(ctx, params, rec, perExchangeCap, result)
+		if processRecommendation(ctx, params, rec, perExchangeCap, result) {
+			// H4: processAutoExchange signalled halt because a ledger write failed
+			// after money moved. Stop processing further recommendations so
+			// subsequent exchanges don't bypass the daily cap.
+			break
+		}
 	}
 
 	return result, nil
@@ -207,7 +212,9 @@ func RunAutoExchange(ctx context.Context, params RunAutoExchangeParams) (*AutoEx
 
 // processRecommendation handles a single reshape recommendation: validates,
 // quotes, and either creates a pending record (manual) or executes (auto).
-func processRecommendation(ctx context.Context, params RunAutoExchangeParams, rec ReshapeRecommendation, perExchangeCap *big.Rat, result *AutoExchangeResult) {
+// Returns true (halt) when processAutoExchange signals that a ledger write
+// failed after money moved and no further exchanges should be attempted.
+func processRecommendation(ctx context.Context, params RunAutoExchangeParams, rec ReshapeRecommendation, perExchangeCap *big.Rat, result *AutoExchangeResult) bool {
 	// Skip idle RIs with no target
 	if rec.TargetInstanceType == "" {
 		result.Skipped = append(result.Skipped, SkippedRecommendation{
@@ -215,19 +222,19 @@ func processRecommendation(ctx context.Context, params RunAutoExchangeParams, re
 			SourceInstanceType: rec.SourceInstanceType,
 			Reason:             "RI is idle (0% utilization) - no target instance type recommended",
 		})
-		return
+		return false
 	}
 
 	offeringID, skip := resolveOffering(ctx, params, rec)
 	if skip != nil {
 		result.Skipped = append(result.Skipped, *skip)
-		return
+		return false
 	}
 
 	quote, skip := getValidatedQuote(ctx, params, rec, offeringID, perExchangeCap)
 	if skip != nil {
 		result.Skipped = append(result.Skipped, *skip)
-		return
+		return false
 	}
 
 	// A nil PaymentDueUSD documents a zero-cost exchange (no payment due),
@@ -245,14 +252,16 @@ func processRecommendation(ctx context.Context, params RunAutoExchangeParams, re
 		} else {
 			result.Pending = append(result.Pending, outcome)
 		}
+		return false
+	}
+
+	outcome, halt := processAutoExchange(ctx, params, rec, offeringID, paymentDueStr, perExchangeCap)
+	if outcome.Error != "" {
+		result.Failed = append(result.Failed, outcome)
 	} else {
-		outcome := processAutoExchange(ctx, params, rec, offeringID, paymentDueStr, perExchangeCap)
-		if outcome.Error != "" {
-			result.Failed = append(result.Failed, outcome)
-		} else {
-			result.Completed = append(result.Completed, outcome)
-		}
+		result.Completed = append(result.Completed, outcome)
 	}
+	return halt
 }
 
 // resolveOffering looks up RI metadata and finds the target offering ID.
@@ -405,12 +414,67 @@ func processManualExchange(ctx context.Context, params RunAutoExchangeParams, re
 	}
 }
 
+// maxLedgerAttempts is the number of times processAutoExchange retries a
+// SaveRIExchangeRecord write after money has moved. Persistent failure halts
+// the run to prevent subsequent exchanges from bypassing the daily cap
+// (GetRIExchangeDailySpend sums the ledger rows that this write would create).
+const maxLedgerAttempts = 3
+
+// chooseEffectiveCap returns the smaller of perExchangeCap and daily headroom
+// (dailyCap - dailySpent). This bounds Execute's MaxPaymentDueUSD so a fresh
+// re-quote cannot accept an amount that exceeds the daily cap (H2 fix).
+func chooseEffectiveCap(dailyCap, dailySpent, perExchangeCap *big.Rat) *big.Rat {
+	remaining := new(big.Rat).Sub(dailyCap, dailySpent)
+	if remaining.Cmp(perExchangeCap) < 0 {
+		return remaining
+	}
+	return perExchangeCap
+}
+
+// acceptedAmountFromQuote returns the payment amount confirmed by a fresh
+// Execute quote, or fallback when freshQ is nil or carries an empty
+// PaymentDueUSDStr. Zero-cost exchanges (PaymentDueRaw empty, AWS returned
+// nil) are recorded as "0" so GetRIExchangeDailySpend's SUM is not distorted
+// by a NULL payment_due in the DB (H3 fix).
+func acceptedAmountFromQuote(freshQ *ExchangeQuoteSummary, fallback string) string {
+	if freshQ == nil {
+		return fallback
+	}
+	if freshQ.PaymentDueUSDStr != "" {
+		return freshQ.PaymentDueUSDStr
+	}
+	return "0"
+}
+
+// saveLedgerRecord saves a completed exchange record with retry, returning a
+// non-nil error if all maxLedgerAttempts fail. Callers treat persistent
+// failure as a halt signal to prevent subsequent exchanges from bypassing the
+// daily cap via a missing ledger row (H4 fix).
+func saveLedgerRecord(ctx context.Context, params RunAutoExchangeParams, record *ExchangeRecord, sourceRIID string) error {
+	var err error
+	for attempt := 1; attempt <= maxLedgerAttempts; attempt++ {
+		err = params.Store.SaveRIExchangeRecord(ctx, record)
+		if err == nil {
+			return nil
+		}
+		if attempt < maxLedgerAttempts {
+			logging.Warnf("ledger save retry %d/%d for %s after money moved: %v",
+				attempt, maxLedgerAttempts, sourceRIID, err)
+		}
+	}
+	return err
+}
+
 // processAutoExchange executes a single exchange in auto mode.
 // If overlapping scheduled runs attempt to exchange the same RI, the first
 // succeeds and the second fails because AWS replaces the source RI atomically.
 // No DB-level mutex is needed — AWS itself guarantees idempotency (an RI can
 // only be exchanged once). The failed attempt is recorded with status=failed.
-func processAutoExchange(ctx context.Context, params RunAutoExchangeParams, rec ReshapeRecommendation, offeringID, paymentDueStr string, perExchangeCap *big.Rat) ExchangeOutcome {
+//
+// Returns (outcome, halt). halt=true means a ledger write failed after money
+// moved; the caller (processRecommendation/RunAutoExchange) must stop further
+// exchanges to preserve cap integrity.
+func processAutoExchange(ctx context.Context, params RunAutoExchangeParams, rec ReshapeRecommendation, offeringID, paymentDueStr string, perExchangeCap *big.Rat) (ExchangeOutcome, bool) {
 	outcome := ExchangeOutcome{
 		SourceRIID:         rec.SourceRIID,
 		SourceInstanceType: rec.SourceInstanceType,
@@ -426,7 +490,7 @@ func processAutoExchange(ctx context.Context, params RunAutoExchangeParams, rec
 	// simulation reflects what would happen if the cap were not a factor.
 	if params.DryRun {
 		outcome.Simulated = true
-		return outcome
+		return outcome, false
 	}
 
 	// Check daily cap
@@ -435,7 +499,7 @@ func processAutoExchange(ctx context.Context, params RunAutoExchangeParams, rec
 		logging.Errorf("daily cap check failed for %s: %v", rec.SourceRIID, err)
 		outcome.Error = fmt.Sprintf("daily cap check failed: %v", err)
 		saveFailedRecord(ctx, params, rec, offeringID, paymentDueStr, outcome.Error, ExchangeModeAuto)
-		return outcome
+		return outcome, false
 	}
 
 	dailyCap := new(big.Rat).SetFloat64(params.Config.MaxPaymentDailyUSD)
@@ -444,7 +508,7 @@ func processAutoExchange(ctx context.Context, params RunAutoExchangeParams, rec
 		logging.Errorf("failed to parse daily spend %q: %v", dailySpendStr, err)
 		outcome.Error = fmt.Sprintf("failed to parse daily spend: %v", err)
 		saveFailedRecord(ctx, params, rec, offeringID, paymentDueStr, outcome.Error, ExchangeModeAuto)
-		return outcome
+		return outcome, false
 	}
 
 	// paymentDueStr is always a decimal here: processRecommendation sets it to
@@ -457,7 +521,7 @@ func processAutoExchange(ctx context.Context, params RunAutoExchangeParams, rec
 		logging.Errorf("failed to parse payment due %q for %s: %v", paymentDueStr, rec.SourceRIID, err)
 		outcome.Error = fmt.Sprintf("failed to parse payment due %q: %v", paymentDueStr, err)
 		saveFailedRecord(ctx, params, rec, offeringID, paymentDueStr, outcome.Error, ExchangeModeAuto)
-		return outcome
+		return outcome, false
 	}
 
 	newTotal := new(big.Rat).Add(dailySpent, paymentDue)
@@ -467,25 +531,35 @@ func processAutoExchange(ctx context.Context, params RunAutoExchangeParams, rec
 		logging.Warnf("skipping exchange for %s: %s", rec.SourceRIID, reason)
 		outcome.Error = reason
 		saveFailedRecord(ctx, params, rec, offeringID, paymentDueStr, reason, ExchangeModeAuto)
-		return outcome
+		return outcome, false
 	}
 
+	// H2: bound Execute's MaxPaymentDueUSD by remaining daily headroom so a
+	// fresh re-quote inside Execute cannot accept an amount that would breach
+	// the daily cap. effectiveCap = min(perExchangeCap, dailyCap - dailySpent).
+	effectiveCap := chooseEffectiveCap(dailyCap, dailySpent, perExchangeCap)
+
 	// Execute the exchange
-	exchangeID, _, execErr := params.ExchangeClient.Execute(ctx, ExchangeExecuteRequest{
+	exchangeID, freshQ, execErr := params.ExchangeClient.Execute(ctx, ExchangeExecuteRequest{
 		Region:           params.Region,
 		ReservedIDs:      []string{rec.SourceRIID},
 		TargetOfferingID: offeringID,
 		TargetCount:      rec.TargetCount,
-		MaxPaymentDueUSD: perExchangeCap,
+		MaxPaymentDueUSD: effectiveCap,
 	})
 
 	if execErr != nil {
 		logging.Errorf("exchange execution failed for %s: %v", rec.SourceRIID, execErr)
 		outcome.Error = execErr.Error()
 		saveFailedRecord(ctx, params, rec, offeringID, paymentDueStr, outcome.Error, ExchangeModeAuto)
-		return outcome
+		return outcome, false
 	}
 
+	// H3: persist the amount AWS actually accepted, not the stale pre-execution
+	// quote. acceptedAmountFromQuote extracts PaymentDueUSDStr from the fresh
+	// Execute quote; falls back to paymentDueStr when freshQ is nil (defensive).
+	accepted := acceptedAmountFromQuote(freshQ, paymentDueStr)
+
 	// Save completed record with ladder linkage when applicable.
 	now := time.Now()
 	record := &ExchangeRecord{
@@ -498,20 +572,28 @@ func processAutoExchange(ctx context.Context, params RunAutoExchangeParams, rec
 		TargetOfferingID:   offeringID,
 		TargetInstanceType: rec.TargetInstanceType,
 		TargetCount:        int(rec.TargetCount),
-		PaymentDue:         paymentDueStr,
+		PaymentDue:         accepted,
 		Status:             "completed",
 		Mode:               string(ExchangeModeAuto),
 		CompletedAt:        &now,
 		LadderRunID:        params.LadderRunID,
 	}
 
-	if err := params.Store.SaveRIExchangeRecord(ctx, record); err != nil {
-		logging.Errorf("failed to save completed exchange record for %s: %v", rec.SourceRIID, err)
+	// Set ExchangeID now so it is present in the outcome even if the ledger
+	// write fails below (callers and logs need it to correlate with AWS).
+	outcome.ExchangeID = exchangeID
+
+	// H4: retry the ledger write via saveLedgerRecord; persistent failure halts
+	// the run so subsequent exchanges don't bypass the cap via missing rows.
+	if saveErr := saveLedgerRecord(ctx, params, record, rec.SourceRIID); saveErr != nil {
+		logging.Errorf("all %d ledger save attempts failed for %s after money moved: %v; halting to prevent cap bypass",
+			maxLedgerAttempts, rec.SourceRIID, saveErr)
+		outcome.Error = fmt.Sprintf("ledger save failed after exchange executed: %v", saveErr)
+		return outcome, true // halt=true: stop processing further exchanges
 	}
 
 	outcome.RecordID = record.ID
-	outcome.ExchangeID = exchangeID
-	return outcome
+	return outcome, false
 }
 
 // ExchangeMode constrains the originating code path of an exchange record so
diff --git a/pkg/exchange/auto_test.go b/pkg/exchange/auto_test.go
index 6c640eed9..f76eb74b9 100644
--- a/pkg/exchange/auto_test.go
+++ b/pkg/exchange/auto_test.go
@@ -24,9 +24,18 @@ type mockExchangeStore struct {
 	// cancelByOriginLast captures the origin argument of the last
 	// CancelPendingExchangesByOrigin call for assertion in scoping tests.
 	cancelByOriginLast *common.ExchangeOrigin
+	// saveErrFor, when non-nil, is called for each SaveRIExchangeRecord call.
+	// Returning a non-nil error simulates a DB write failure for that record.
+	// Use this to inject ledger-write failures without affecting other saves.
+	saveErrFor func(record *ExchangeRecord) error
 }
 
 func (m *mockExchangeStore) SaveRIExchangeRecord(_ context.Context, record *ExchangeRecord) error {
+	if m.saveErrFor != nil {
+		if err := m.saveErrFor(record); err != nil {
+			return err
+		}
+	}
 	if record.ID == "" {
 		record.ID = fmt.Sprintf("test-id-%d", len(m.savedRecords))
 	}
@@ -114,15 +123,27 @@ type mockExchangeClient struct {
 	executeResult string
 	executeErr    error
 	executeCalls  int
+	// executeRequests captures each ExchangeExecuteRequest passed to Execute.
+	// Use this to assert that MaxPaymentDueUSD is bounded correctly (H2).
+	executeRequests []ExchangeExecuteRequest
+	// executeQuoteResult, when non-nil, is returned as the quote from Execute
+	// instead of quoteResult. Use this to simulate a fresh quote whose amount
+	// differs from the pre-execution GetQuote result (H3 test).
+	executeQuoteResult *ExchangeQuoteSummary
 }
 
 func (m *mockExchangeClient) GetQuote(_ context.Context, _ ExchangeQuoteRequest) (*ExchangeQuoteSummary, error) {
 	return m.quoteResult, m.quoteErr
 }
 
-func (m *mockExchangeClient) Execute(_ context.Context, _ ExchangeExecuteRequest) (string, *ExchangeQuoteSummary, error) {
+func (m *mockExchangeClient) Execute(_ context.Context, req ExchangeExecuteRequest) (string, *ExchangeQuoteSummary, error) {
 	m.executeCalls++
-	return m.executeResult, m.quoteResult, m.executeErr
+	m.executeRequests = append(m.executeRequests, req)
+	q := m.quoteResult
+	if m.executeQuoteResult != nil {
+		q = m.executeQuoteResult
+	}
+	return m.executeResult, q, m.executeErr
 }
 
 func defaultQuote() *ExchangeQuoteSummary {
@@ -343,10 +364,11 @@ func TestProcessAutoExchange_UnparseablePaymentDue_FailsClosed(t *testing.T) {
 			}
 			perExchangeCap := new(big.Rat).SetFloat64(params.Config.MaxPaymentPerExchangeUSD)
 
-			outcome := processAutoExchange(context.Background(), params, rec, "offering-123", paymentDueStr, perExchangeCap)
+			outcome, halt := processAutoExchange(context.Background(), params, rec, "offering-123", paymentDueStr, perExchangeCap)
 
 			assert.Contains(t, outcome.Error, "failed to parse payment due")
 			assert.Empty(t, outcome.ExchangeID)
+			assert.False(t, halt, "parse failure should not halt subsequent exchanges")
 			assert.Zero(t, client.executeCalls, "exchange must not execute on unparseable payment due")
 
 			// A failed record must be persisted for the audit trail,
@@ -554,3 +576,244 @@ func TestRunAutoExchange_NoLadderRunID_RecordHasNilLadderRunID(t *testing.T) {
 	assert.Nil(t, store.savedRecords[0].LadderRunID,
 		"standalone run must save record with nil LadderRunID")
 }
+
+// ─── H2: effective cap bounded by daily headroom ──────────────────────────────
+
+// TestProcessAutoExchange_EffectiveCap_BoundedByDailyHeadroom is the regression
+// test for H2: Execute's MaxPaymentDueUSD must be the minimum of perExchangeCap
+// and (dailyCap - dailySpent) so a fresh re-quote cannot accept an amount that
+// would exceed daily headroom.
+//
+// Setup: dailyCap=$500, dailySpent=$450, perExchangeCap=$100.
+// Headroom = $500-$450 = $50 < $100 perExchangeCap.
+// Expected: MaxPaymentDueUSD passed to Execute = $50 (headroom).
+func TestProcessAutoExchange_EffectiveCap_BoundedByDailyHeadroom(t *testing.T) {
+	t.Parallel()
+
+	due, _ := ParseDecimalRat("40.000000")
+	store := &mockExchangeStore{dailySpend: "450.00"}
+	client := &mockExchangeClient{
+		quoteResult:   defaultQuote(),
+		executeResult: "exch-h2-test",
+		// The fresh Execute quote is $40, which is under the effective cap ($50).
+		executeQuoteResult: &ExchangeQuoteSummary{
+			IsValidExchange:  true,
+			PaymentDueRaw:    "40.000000",
+			PaymentDueUSD:    due,
+			PaymentDueUSDStr: "40.000000",
+			CurrencyCode:     "USD",
+		},
+	}
+	params := defaultParams(store, client)
+	params.Config.Mode = "auto"
+	params.Config.MaxPaymentDailyUSD = 500.0
+	params.Config.MaxPaymentPerExchangeUSD = 100.0
+
+	rec := ReshapeRecommendation{
+		SourceRIID:         "ri-001",
+		SourceInstanceType: "m5.xlarge",
+		TargetInstanceType: "m5.large",
+		SourceCount:        1,
+		TargetCount:        2,
+		UtilizationPercent: 50.0,
+	}
+	perExchangeCap := new(big.Rat).SetFloat64(params.Config.MaxPaymentPerExchangeUSD)
+
+	outcome, halt := processAutoExchange(context.Background(), params, rec, "offering-123", "40.000000", perExchangeCap)
+
+	require.Empty(t, outcome.Error, "exchange should succeed within effective cap")
+	assert.False(t, halt)
+	assert.Equal(t, "exch-h2-test", outcome.ExchangeID)
+
+	// Assert that Execute was called with MaxPaymentDueUSD = $50 (headroom),
+	// not $100 (perExchangeCap).
+	require.Len(t, client.executeRequests, 1)
+	gotCap := client.executeRequests[0].MaxPaymentDueUSD
+	require.NotNil(t, gotCap)
+	expectedCap := new(big.Rat) // $50
+	expectedCap.SetFrac64(50, 1)
+	assert.Equal(t, 0, gotCap.Cmp(expectedCap),
+		"MaxPaymentDueUSD passed to Execute must equal dailyHeadroom ($50), got $%s", gotCap.FloatString(2))
+}
+
+// TestProcessAutoExchange_EffectiveCap_UsesPerExchangeCapWhenSmaller verifies
+// that when perExchangeCap < dailyHeadroom, perExchangeCap is used as-is.
+func TestProcessAutoExchange_EffectiveCap_UsesPerExchangeCapWhenSmaller(t *testing.T) {
+	t.Parallel()
+
+	store := &mockExchangeStore{dailySpend: "0"} // headroom = $500
+	client := &mockExchangeClient{
+		quoteResult:   defaultQuote(),
+		executeResult: "exch-h2b-test",
+	}
+	params := defaultParams(store, client)
+	params.Config.Mode = "auto"
+	params.Config.MaxPaymentDailyUSD = 500.0
+	params.Config.MaxPaymentPerExchangeUSD = 100.0
+
+	rec := ReshapeRecommendation{
+		SourceRIID:         "ri-001",
+		SourceInstanceType: "m5.xlarge",
+		TargetInstanceType: "m5.large",
+		SourceCount:        1,
+		TargetCount:        2,
+		UtilizationPercent: 50.0,
+	}
+	perExchangeCap := new(big.Rat).SetFloat64(params.Config.MaxPaymentPerExchangeUSD)
+
+	_, halt := processAutoExchange(context.Background(), params, rec, "offering-123", "0.000000", perExchangeCap)
+	assert.False(t, halt)
+
+	require.Len(t, client.executeRequests, 1)
+	gotCap := client.executeRequests[0].MaxPaymentDueUSD
+	require.NotNil(t, gotCap)
+	// Headroom ($500) > perExchangeCap ($100), so effectiveCap = perExchangeCap.
+	expectedCap := new(big.Rat).SetFloat64(100.0)
+	assert.Equal(t, 0, gotCap.Cmp(expectedCap),
+		"MaxPaymentDueUSD must equal perExchangeCap ($100) when headroom is larger, got $%s", gotCap.FloatString(2))
+}
+
+// ─── H3: accepted amount from fresh Execute quote ─────────────────────────────
+
+// TestProcessAutoExchange_AcceptedAmountFromFreshQuote is the regression test
+// for H3: the completed record's PaymentDue must reflect the amount AWS
+// confirmed during Execute, not the stale pre-execution GetQuote amount.
+func TestProcessAutoExchange_AcceptedAmountFromFreshQuote(t *testing.T) {
+	t.Parallel()
+
+	preQuoteDue, _ := ParseDecimalRat("30.000000")
+	freshDue, _ := ParseDecimalRat("35.500000") // AWS re-quoted higher (within cap)
+
+	store := &mockExchangeStore{dailySpend: "0"}
+	client := &mockExchangeClient{
+		quoteResult: &ExchangeQuoteSummary{
+			IsValidExchange:  true,
+			PaymentDueRaw:    "30.000000",
+			PaymentDueUSD:    preQuoteDue,
+			PaymentDueUSDStr: "30.000000",
+			CurrencyCode:     "USD",
+		},
+		executeResult: "exch-h3-test",
+		// Execute returns a higher accepted amount.
+		executeQuoteResult: &ExchangeQuoteSummary{
+			IsValidExchange:  true,
+			PaymentDueRaw:    "35.500000",
+			PaymentDueUSD:    freshDue,
+			PaymentDueUSDStr: "35.500000",
+			CurrencyCode:     "USD",
+		},
+	}
+	params := defaultParams(store, client)
+	params.Config.Mode = "auto"
+	params.Config.MaxPaymentDailyUSD = 500.0
+	params.Config.MaxPaymentPerExchangeUSD = 100.0
+
+	rec := ReshapeRecommendation{
+		SourceRIID:         "ri-001",
+		SourceInstanceType: "m5.xlarge",
+		TargetInstanceType: "m5.large",
+		SourceCount:        1,
+		TargetCount:        2,
+		UtilizationPercent: 50.0,
+	}
+	perExchangeCap := new(big.Rat).SetFloat64(params.Config.MaxPaymentPerExchangeUSD)
+
+	outcome, halt := processAutoExchange(context.Background(), params, rec, "offering-123", "30.000000", perExchangeCap)
+
+	require.Empty(t, outcome.Error)
+	assert.False(t, halt)
+
+	// The saved record must carry the FRESH accepted amount, not the pre-quote.
+	require.Len(t, store.savedRecords, 1)
+	assert.Equal(t, "35.500000", store.savedRecords[0].PaymentDue,
+		"completed record must store the amount AWS actually accepted, not the pre-execution quote")
+}
+
+// ─── H4: halt on persistent ledger-save failure ──────────────────────────────
+
+// TestProcessAutoExchange_LedgerSaveFailure_HaltsAndReturnsError is the
+// regression test for H4: when SaveRIExchangeRecord fails for a completed
+// exchange (money has already moved), processAutoExchange must retry and, on
+// persistent failure, return halt=true to stop further exchanges.
+func TestProcessAutoExchange_LedgerSaveFailure_HaltsAndReturnsError(t *testing.T) {
+	t.Parallel()
+
+	store := &mockExchangeStore{
+		dailySpend: "0",
+		saveErrFor: func(r *ExchangeRecord) error {
+			if r.Status == "completed" {
+				return fmt.Errorf("DB connection refused")
+			}
+			return nil
+		},
+	}
+	client := &mockExchangeClient{
+		quoteResult:   defaultQuote(),
+		executeResult: "exch-h4-test",
+	}
+	params := defaultParams(store, client)
+	params.Config.Mode = "auto"
+
+	rec := ReshapeRecommendation{
+		SourceRIID:         "ri-001",
+		SourceInstanceType: "m5.xlarge",
+		TargetInstanceType: "m5.large",
+		SourceCount:        1,
+		TargetCount:        2,
+		UtilizationPercent: 50.0,
+	}
+	perExchangeCap := new(big.Rat).SetFloat64(params.Config.MaxPaymentPerExchangeUSD)
+
+	outcome, halt := processAutoExchange(context.Background(), params, rec, "offering-123", "0.000000", perExchangeCap)
+
+	// Execute succeeded but ledger write failed: outcome must carry an error
+	// and halt must be true to prevent cap bypass.
+	assert.Contains(t, outcome.Error, "ledger save failed")
+	assert.Equal(t, "exch-h4-test", outcome.ExchangeID,
+		"ExchangeID must be set even when ledger write fails (money moved)")
+	assert.True(t, halt, "persistent ledger failure must signal halt to stop further exchanges")
+}
+
+// TestRunAutoExchange_LedgerSaveFailure_StopsAfterFirstExchange verifies that
+// when the first exchange's ledger write fails (H4), RunAutoExchange does not
+// proceed to a second exchange recommendation.
+func TestRunAutoExchange_LedgerSaveFailure_StopsAfterFirstExchange(t *testing.T) {
+	t.Parallel()
+
+	store := &mockExchangeStore{
+		dailySpend: "0",
+		saveErrFor: func(r *ExchangeRecord) error {
+			if r.Status == "completed" {
+				return fmt.Errorf("DB write failed")
+			}
+			return nil
+		},
+	}
+	client := &mockExchangeClient{
+		quoteResult:   defaultQuote(),
+		executeResult: "exch-stop-test",
+	}
+	params := defaultParams(store, client)
+	params.Config.Mode = "auto"
+	// Add a second RI so there are two recommendations.
+	params.RIs = append(params.RIs, RIInfo{
+		ID: "ri-002", InstanceType: "m5.xlarge", InstanceCount: 1,
+		OfferingClass: "convertible", NormalizationFactor: 8,
+	})
+	params.Utilization = append(params.Utilization,
+		UtilizationInfo{RIID: "ri-002", UtilizationPercent: 50.0})
+	params.RIMetadata["ri-002"] = RIMetadataInfo{
+		ProductDescription: "Linux/UNIX", InstanceTenancy: "default",
+		Scope: "Region", Duration: 31536000,
+	}
+
+	result, err := RunAutoExchange(context.Background(), params)
+	require.NoError(t, err)
+
+	// First exchange executes but ledger fails -> appears in Failed.
+	// Second exchange must NOT have been attempted.
+	assert.Len(t, result.Failed, 1, "exactly one failed outcome (the ledger failure)")
+	assert.Empty(t, result.Completed, "no completed outcomes when ledger fails")
+	assert.Equal(t, 1, client.executeCalls,
+		"Execute must be called exactly once; second exchange must be halted")
+}

From 0acb4abf32c578dab122e47f08de1646891ed54c Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Fri, 17 Jul 2026 21:35:32 +0300
Subject: [PATCH 2/2] fix(exchange): handle ParseDecimalRat error, extract
 checkCapsAndComputeHeadroom

Fixes golangci-lint errcheck failure on handler_ri_exchange.go:1351 where
`exchange.ParseDecimalRat` error was discarded with `_`. The fix extracts a
`checkCapsAndComputeHeadroom` helper that bundles the three cap-validation
checks (MaxDailyUSD==0, checkDailyCap, MaxPerExchangeUSD==0) and the headroom
calculation, properly returning a reason string on any parse failure (fail
closed). This also brings `executeApprovedExchange` back under gocyclo -over 10
(complexity 8) after the helper reduces it from the 12 that errcheck enforcement
would have caused.

Rebased onto main (after PR #1456 merged; no conflicts in store_postgres.go).
---
 internal/api/handler_ri_exchange.go      | 46 +++++++++++++++---------
 internal/api/handler_ri_exchange_test.go |  2 +-
 2 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/internal/api/handler_ri_exchange.go b/internal/api/handler_ri_exchange.go
index e7c945e41..bdaada9eb 100644
--- a/internal/api/handler_ri_exchange.go
+++ b/internal/api/handler_ri_exchange.go
@@ -1314,6 +1314,32 @@ func handlerAcceptedAmount(freshQ *exchange.ExchangeQuoteSummary, fallback strin
 	return "0"
 }
 
+// checkCapsAndComputeHeadroom validates the spending-cap configuration, runs the
+// daily-cap check, and computes the effective MaxPaymentDueUSD that Execute must
+// not exceed (H2: remaining daily headroom vs per-exchange cap, whichever is
+// smaller). Returns a non-empty reason string on any failure so the caller can
+// forward it to failExchange.
+func checkCapsAndComputeHeadroom(dailySpendStr, paymentDue string, cfg *config.GlobalConfig) (effectiveCap *big.Rat, reason string) {
+	if cfg.RIExchangeMaxDailyUSD == 0 {
+		return nil, "daily spending cap is not configured (RIExchangeMaxDailyUSD is 0)"
+	}
+	if reason := checkDailyCap(dailySpendStr, paymentDue, cfg.RIExchangeMaxDailyUSD); reason != "" {
+		return nil, reason
+	}
+	if cfg.RIExchangeMaxPerExchangeUSD == 0 {
+		return nil, "per-exchange spending cap is not configured (RIExchangeMaxPerExchangeUSD is 0)"
+	}
+	// checkDailyCap already verified dailySpendStr is parseable; a second failure
+	// is an internal error - fail closed to avoid executing with wrong headroom.
+	dailySpent, err := exchange.ParseDecimalRat(dailySpendStr)
+	if err != nil || dailySpent == nil {
+		return nil, fmt.Sprintf("daily spend re-parse failed (internal error): %v", err)
+	}
+	dailyCap := new(big.Rat).SetFloat64(cfg.RIExchangeMaxDailyUSD)
+	perExchangeCap := new(big.Rat).SetFloat64(cfg.RIExchangeMaxPerExchangeUSD)
+	return handlerChooseEffectiveCap(dailyCap, dailySpent, perExchangeCap), ""
+}
+
 // executeApprovedExchange checks caps and executes the exchange after approval.
 func (h *Handler) executeApprovedExchange(ctx context.Context, id string, record *config.RIExchangeRecord) (any, error) {
 	dailySpendStr, err := h.config.GetRIExchangeDailySpend(ctx, time.Now())
@@ -1326,13 +1352,6 @@ func (h *Handler) executeApprovedExchange(ctx context.Context, id string, record
 		return h.failExchange(ctx, id, "config load failed")
 	}
 
-	if globalCfg.RIExchangeMaxDailyUSD == 0 {
-		return h.failExchange(ctx, id, "daily spending cap is not configured (RIExchangeMaxDailyUSD is 0)")
-	}
-	if reason := checkDailyCap(dailySpendStr, record.PaymentDue, globalCfg.RIExchangeMaxDailyUSD); reason != "" {
-		return h.failExchange(ctx, id, reason)
-	}
-
 	region := record.Region
 	if region == "" {
 		// Region is a required field captured at record-creation time.
@@ -1341,18 +1360,11 @@ func (h *Handler) executeApprovedExchange(ctx context.Context, id string, record
 		return h.failExchange(ctx, id, "exchange record has no region; cannot execute safely")
 	}
 
-	if globalCfg.RIExchangeMaxPerExchangeUSD == 0 {
-		return h.failExchange(ctx, id, "per-exchange spending cap is not configured (RIExchangeMaxPerExchangeUSD is 0)")
+	effectiveCap, reason := checkCapsAndComputeHeadroom(dailySpendStr, record.PaymentDue, globalCfg)
+	if reason != "" {
+		return h.failExchange(ctx, id, reason)
 	}
 
-	// H2: bound Execute's MaxPaymentDueUSD by remaining daily headroom so a
-	// fresh re-quote inside Execute cannot accept an amount that would breach
-	// the daily cap. dailySpend was verified parseable by checkDailyCap above.
-	dailySpent, _ := exchange.ParseDecimalRat(dailySpendStr)
-	dailyCap := new(big.Rat).SetFloat64(globalCfg.RIExchangeMaxDailyUSD)
-	perExchangeCap := new(big.Rat).SetFloat64(globalCfg.RIExchangeMaxPerExchangeUSD)
-	effectiveCap := handlerChooseEffectiveCap(dailyCap, dailySpent, perExchangeCap)
-
 	execFn := exchange.ExecuteExchange
 	if h.executeExchangeFn != nil {
 		execFn = h.executeExchangeFn
diff --git a/internal/api/handler_ri_exchange_test.go b/internal/api/handler_ri_exchange_test.go
index 15804c633..08d674568 100644
--- a/internal/api/handler_ri_exchange_test.go
+++ b/internal/api/handler_ri_exchange_test.go
@@ -1773,5 +1773,5 @@ func TestExecuteApprovedExchange_LedgerWriteFailure_ReturnsError(t *testing.T) {
 	assert.Contains(t, err.Error(), "ledger update failed",
 		"error must describe the ledger failure for operator triage")
 	assert.Contains(t, err.Error(), "exch-h4-test",
-		"error must include the exchange ID so operators can correlate with AWS")
+		"error must include the exchange ID for operator correlation with AWS")
 }