fix(purchase): show the RI exchange guard and outcome notes in History

internal/api/handler_ri_exchange_history_notes_test.go

@@ -0,0 +1,48 @@
+package api
+
+import (
+	"context"
+	"testing"
+
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
+	"github.com/aws/aws-lambda-go/events"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+)
+
+// #1050's two RI exchange notes reach the History row as written: the
+// source-RI guard's refusal and the outcome-unknown resolve note. A raw
+// provider error on another row is still summarized.
+func TestHandler_getRIExchangeHistory_ShowsRIExchangeNotes(t *testing.T) {
+	const (
+		sourceNote  = "RI exchange source RI is held by an exchange in progress: source RI(s) ri-0a1b2c3d already have an exchange in progress; resolve that exchange first"
+		outcomeNote = "outcome unknown: the provider returned an error; verify the source RIs and target reservation at AWS, then POST /api/ri-exchange/resolve/550e8400-e29b-41d4-a716-000000010501"
+		rawProvider = "RequestError: send request failed for arn:aws:ec2:us-east-1:123456789012:reserved-instances/ri-leak"
+	)
+	ctx := context.Background()
+	mockAuth := new(MockAuthService)
+	t.Cleanup(func() { mockAuth.AssertExpectations(t) })
+	mockAuth.On("ValidateSession", ctx, "tok").Return(&Session{UserID: "uid"}, nil)
+	mockAuth.grantScoped("u-a")
+	store := new(MockConfigStore)
+	store.ListCloudAccountsFn = func(_ context.Context, _ config.CloudAccountFilter) ([]config.CloudAccount, error) {
+		return []config.CloudAccount{{ID: "u-a", Name: "a", Provider: "aws", ExternalID: exchangeAcctA}}, nil
+	}
+	store.On("GetRIExchangeHistoryForAccounts", ctx, mock.Anything, 500, mock.Anything).Return([]config.RIExchangeRecord{
+		{ID: "source", AccountID: exchangeAcctA, Status: "failed", Error: sourceNote},
+		{ID: "outcome", AccountID: exchangeAcctA, Status: "processing", Error: outcomeNote},
+		{ID: "raw", AccountID: exchangeAcctA, Status: "failed", Error: rawProvider},
+	}, nil).Once()
+	h := &Handler{auth: mockAuth, config: store}
+
+	result, err := h.getRIExchangeHistory(ctx, &events.LambdaFunctionURLRequest{Headers: map[string]string{"Authorization": "Bearer tok"}})
+	require.NoError(t, err)
+	got := map[string]string{}
+	for _, r := range result.(*RIExchangeHistoryResponse).Records {
+		got[r.ID] = r.Error
+	}
+	assert.Equal(t, sourceNote, got["source"])
+	assert.Equal(t, outcomeNote, got["outcome"])
+	assert.NotContains(t, got["raw"], "ri-leak", "a raw provider error is still summarized")
+}

internal/api/handler_ri_exchange_outcome_unknown_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/LeanerCloud/cloud-commitments-go/pkg/exchange"
 	"github.com/LeanerCloud/cloud-commitments-platform/internal/auth"
 	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/purchase"
 	"github.com/aws/aws-lambda-go/events"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
@@ -52,6 +53,7 @@ func assertOutcomeUnknown(t *testing.T, store *MockConfigStore, id string, resp
 		assert.Contains(t, text, "/api/ri-exchange/resolve/"+id, "%s must tell the operator how to resolve it", name)
 		assert.NotContains(t, text, outcomeUnknownLeak, "%s must not carry raw provider text", name)
 	}
+	assert.Equal(t, note, purchase.SanitizeStoredError(note), "History shows the stored note as written")
 	store.AssertNotCalled(t, "FailRIExchange", mock.Anything, mock.Anything, mock.Anything)
 	store.AssertNotCalled(t, "CompleteRIExchangeWithPayment", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
 }

internal/purchase/errnotes.go

@@ -68,9 +68,15 @@ var bareNotePatterns = []string{
 	`exchange reservation returned invalid ceiling "` + shortValuePattern + `"`,
 	// config.refuseRIExchangeSourceInFlightDirect refusal, stored whole from
 	// err.Error() (sentinel text first); only the shared RI ids vary.
-	`RI exchange source RI is held by an exchange in progress: source RI\(s\) ` + directGuardIDListPattern + ` are held by a direct exchange that may have reached AWS; an operator must verify the outcome at AWS and settle the record`,
+	`RI exchange source RI is held by an exchange in progress: source RI\(s\) ` + riIDListPattern + ` are held by a direct exchange that may have reached AWS; an operator must verify the outcome at AWS and settle the record`,
 	// Scheduler note for a provider name it does not know.
 	`unknown provider "` + shortValuePattern + `" \(valid: aws, azure, gcp\)`,
+	// The source-RI guard's refusal (config refuseRIExchangeSourceInFlight): only
+	// the shared RI ids, never the blocking exchange (#1050, #1428).
+	`RI exchange source RI is held by an exchange in progress: source RI\(s\) ` + riIDListPattern + ` already have an exchange in progress; resolve that exchange first`,
+	// The outcome-unknown note (api leaveExchangeOutcomeUnknown): fixed text, the
+	// sanitized SummarizeFailure reason and the exchange id to resolve (#1050).
+	`outcome unknown: (?:` + joinAlt(append(categoryNotePatterns(), regexp.QuoteMeta(outcomeUnconfirmedMessage))) + `); verify the source RIs and target reservation at AWS, then POST /api/ri-exchange/resolve/(?i:` + uuidPattern + `)`,
 }
 
 // categoryNotePatterns matches a collection failure stored as "<provider>:
@@ -95,15 +101,14 @@ const (
 	commitmentIDPattern = `[A-Za-z0-9._:/-]{1,256}`
 	identityKeyPattern  = `[A-Za-z0-9._:/+-]{1,256}`
 	idListPattern       = identityKeyPattern + `(?:, ` + identityKeyPattern + `)*`
+	// riIDListPattern is the ", "-joined source RI ids of an RI exchange.
+	riIDListPattern = `[A-Za-z0-9-]{1,64}(?:, [A-Za-z0-9-]{1,64})*`
 	// quotedTextPattern is the body of a Go %q string: printable characters
 	// other than '"' and '\\', ASCII space, or an escape.
 	quotedTextPattern  = `(?:[^"\\\p{C}\p{Z}]| |\\[ -~])*`
 	skippedNotePattern = `\d+ recommendation\(s\) without a frozen baseline skipped \(under-buy\): ` + idListPattern
 )
 
-// directGuardIDListPattern is the ", "-joined lowercase RI ids the direct-execute guard names.
-const directGuardIDListPattern = `[A-Za-z0-9-]{1,64}(?:, [A-Za-z0-9-]{1,64})*`
-
 // serverNoteHeads are server notes whose tail can carry a wrapped store or
 // config error. Only the fixed head is ever shown; the tail is dropped.
 var serverNoteHeads = []string{

internal/purchase/errnotes_ri_exchange_test.go

@@ -0,0 +1,64 @@
+package purchase
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// The two #1050 RI exchange notes are stored as the record's error and read
+// back through SanitizeStoredError on History, so they must pass verbatim,
+// and anything that is not their closed shape must still fall back.
+const (
+	riExchangeSourceInFlightNote = "RI exchange source RI is held by an exchange in progress: source RI(s) ri-0a1b2c3d, 1ba8e2e3-0c4f-4a7e-9a7b-5d6e7f8a9b0c already have an exchange in progress; resolve that exchange first"
+	riExchangeOutcomeUnknownHead = "outcome unknown: "
+	riExchangeOutcomeUnknownTail = "; verify the source RIs and target reservation at AWS, then POST /api/ri-exchange/resolve/550e8400-e29b-41d4-a716-000000010501"
+)
+
+func TestSanitizeStoredError_RIExchangeNotesPassVerbatim(t *testing.T) {
+	reasons := []string{
+		ErrCategoryMessage(ErrCategoryUnknown),
+		ErrCategoryMessage(ErrCategoryAccessDenied),
+		ErrCategoryMessage(ErrCategoryUnavailable),
+		ErrCategoryMessage(ErrCategoryLimit),
+		outcomeUnconfirmedMessage,
+	}
+	notes := make([]string, 0, len(reasons)+1)
+	notes = append(notes, riExchangeSourceInFlightNote)
+	for _, reason := range reasons {
+		notes = append(notes, riExchangeOutcomeUnknownHead+reason+riExchangeOutcomeUnknownTail)
+	}
+	// The resolve route accepts an upper-case id, so the note may carry one.
+	notes = append(notes, riExchangeOutcomeUnknownHead+outcomeUnconfirmedMessage+riExchangeOutcomeUnknownTail[:len(riExchangeOutcomeUnknownTail)-36]+strings.ToUpper(riExchangeOutcomeUnknownTail[len(riExchangeOutcomeUnknownTail)-36:]))
+	notes = append(notes, sourceInFlightNote(strings.Repeat("a", 64)))
+	for _, note := range notes {
+		assert.Equal(t, note, SanitizeStoredError(note))
+	}
+}
+
+func TestSanitizeStoredError_RIExchangeNotesStayClosed(t *testing.T) {
+	leak := "arn:aws:ec2:us-east-1:123456789012:reserved-instances/ri-leak"
+	for _, in := range []string{
+		// A raw provider reason in place of a summarized one.
+		riExchangeOutcomeUnknownHead + "RequestError: send request failed for " + leak + riExchangeOutcomeUnknownTail,
+		// A resolve id that is not a UUID.
+		riExchangeOutcomeUnknownHead + ErrCategoryMessage(ErrCategoryUnknown) + "; verify the source RIs and target reservation at AWS, then POST /api/ri-exchange/resolve/" + leak,
+		// RI ids carrying text the guard never writes.
+		"RI exchange source RI is held by an exchange in progress: source RI(s) ri-1 " + leak + " already have an exchange in progress; resolve that exchange first",
+		// Text ahead of either note (the patterns are anchored at the head).
+		leak + " " + riExchangeSourceInFlightNote,
+		leak + ": " + riExchangeOutcomeUnknownHead + outcomeUnconfirmedMessage + riExchangeOutcomeUnknownTail,
+		// An RI id past 64 characters, and an empty RI list.
+		sourceInFlightNote(strings.Repeat("a", 65)),
+		sourceInFlightNote(""),
+	} {
+		out := SanitizeStoredError(in)
+		assert.NotEqual(t, in, out, in)
+		assert.NotContains(t, out, leak)
+	}
+}
+
+func sourceInFlightNote(ids string) string {
+	return "RI exchange source RI is held by an exchange in progress: source RI(s) " + ids + " already have an exchange in progress; resolve that exchange first"
+}