From 34462fc79284c7509beecceb4f5dfb7386edf178 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 4 Nov 2026 20:58:53 +0100 Subject: [PATCH 1/4] fix(history): recognize the executor's stored AWS refusals in the duplicate-warning gate The executor stores AWS failures as ": purchase failed: : operation error ...", so the stored-row check never matched and the duplicate-purchase hint always stayed. The gate now accepts a closed set of known heads (resource token, the executor's "purchase failed: " wrapper, and one exact AWS provider library head per service) before "operation error"; any other text still keeps the hint. A rec with an empty provider no longer counts as AWS. The cfn-lint hook was skipped locally: cfn-lint 1.58.0 does not install on Python 3.15, and this change touches no CloudFormation template. Closes #1694 Closes #1725 Co-Authored-By: Claude Opus 5.5 --- internal/api/history_duplicate_hint_test.go | 25 ++++++++++ internal/purchase/errsummary.go | 49 ++++++++++++++++--- internal/purchase/errsummary_provider_test.go | 16 +++++- 3 files changed, 81 insertions(+), 9 deletions(-) diff --git a/internal/api/history_duplicate_hint_test.go b/internal/api/history_duplicate_hint_test.go index d145463f4..d625a6a03 100644 --- a/internal/api/history_duplicate_hint_test.go +++ b/internal/api/history_duplicate_hint_test.go @@ -51,3 +51,28 @@ func TestHistoryRow_DuplicateHintKeptOutsideDefiniteAWSRefusals(t *testing.T) { assert.Contains(t, row.OpsHint, "duplicate guard", c.name) } } + +func TestHistoryRow_ExecutorStoredAWSRefusalDropsDuplicateHint(t *testing.T) { + const ec2Stored = "m5.large: purchase failed: failed to purchase EC2 RI: operation error EC2: PurchaseReservedInstancesOffering, https response error StatusCode: 403, RequestID: 11111111-2222-3333-4444-555555555555, api error UnauthorizedOperation: You are not authorized (account 123456789012)" + const rdsStored = "db.r6g.large: purchase failed: failed to purchase RDS RI: operation error RDS: PurchaseReservedDBInstancesOffering, https response error StatusCode: 403, RequestID: 11111111-2222-3333-4444-555555555555, api error AccessDenied: denied" + ec2 := config.RecommendationRecord{Provider: "aws", Service: "ec2"} + redshift := config.RecommendationRecord{Provider: "aws", Service: "redshift"} + for name, stored := range map[string]string{"ec2": ec2Stored, "rds beside ec2": ec2Stored + "; " + rdsStored} { + row := executionToHistoryRow(config.PurchaseExecution{Status: "failed", Error: stored, Recommendations: []config.RecommendationRecord{ec2, redshift}}, "", "") + assert.NotContains(t, row.OpsHint, "duplicate guard", name) + assert.NotEmpty(t, row.OpsHint, name+": ops_hint stays set so Retry stays hidden") + assert.True(t, row.RedriveUnsafe, name) + } + for name, c := range map[string]struct { + recs []config.RecommendationRecord + stored string + }{ + "unknown head": {[]config.RecommendationRecord{ec2}, strings.Replace(ec2Stored, "failed to purchase EC2 RI", "failed to purchase GPU RI", 1)}, + "5xx": {[]config.RecommendationRecord{ec2}, strings.Replace(ec2Stored, "StatusCode: 403", "StatusCode: 503", 1)}, + "empty provider": {[]config.RecommendationRecord{{Service: "ec2"}}, fakeAWSDenied}, + "mixed run": {[]config.RecommendationRecord{ec2}, ec2Stored + "; t4g.nano: purchase failed: purchase outcome unknown"}, + } { + row := executionToHistoryRow(config.PurchaseExecution{Status: "failed", Error: c.stored, Recommendations: c.recs}, "", "") + assert.Contains(t, row.OpsHint, "duplicate guard", name) + } +} diff --git a/internal/purchase/errsummary.go b/internal/purchase/errsummary.go index f218996ba..9ff591bc2 100644 --- a/internal/purchase/errsummary.go +++ b/internal/purchase/errsummary.go @@ -148,13 +148,14 @@ func classifyCode(code string) string { // Explicit code fields in the providers' own error formats. Words elsewhere // (URLs, resource names, bodies) never count. var ( - awsCodeRE = regexp.MustCompile(`api error ([A-Za-z0-9_.]+):`) - awsStatusRE = regexp.MustCompile(`StatusCode: (\d{3})`) - azCodeRE = regexp.MustCompile(`ERROR CODE: ([A-Za-z0-9_.-]+)`) - azStatusRE = regexp.MustCompile(`RESPONSE (\d{3}):`) - gcpReasonRE = regexp.MustCompile(`Reason: ([A-Za-z]+), Message:`) - gcpErrorRE = regexp.MustCompile(`googleapi: Error (\d{3}): [^\n]*?(?:, ([A-Za-z]+))?(?:\n|$)`) - definiteAWSRE = regexp.MustCompile(`^(?:[A-Za-z0-9][A-Za-z0-9._-]*: )?operation error [^\n]*StatusCode: (4\d{2}),[^\n]*api error ([A-Za-z0-9_.]+): [^\n]*$`) + awsCodeRE = regexp.MustCompile(`api error ([A-Za-z0-9_.]+):`) + awsStatusRE = regexp.MustCompile(`StatusCode: (\d{3})`) + azCodeRE = regexp.MustCompile(`ERROR CODE: ([A-Za-z0-9_.-]+)`) + azStatusRE = regexp.MustCompile(`RESPONSE (\d{3}):`) + gcpReasonRE = regexp.MustCompile(`Reason: ([A-Za-z]+), Message:`) + gcpErrorRE = regexp.MustCompile(`googleapi: Error (\d{3}): [^\n]*?(?:, ([A-Za-z]+))?(?:\n|$)`) + resourceHeadRE = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._-]*: `) + definiteAWSRE = regexp.MustCompile(`^operation error [^\n]*StatusCode: (4\d{2}),[^\n]*api error ([A-Za-z0-9_.]+): [^\n]*$`) ) func statusOf(re *regexp.Regexp, s string) int { @@ -214,10 +215,42 @@ var definiteAWSCodes = map[string]bool{ "ReservedDBInstancesOfferingNotFound": true, "ReservedCacheNodesOfferingNotFound": true, } +// executorPurchaseFailedHead is the wrapper the executor adds around a provider +// error (internal/purchase/execution.go, "purchase failed: %w"). +const executorPurchaseFailedHead = "purchase failed: " + +// awsLibraryHeads are the exact heads the AWS provider library puts in front of +// the SDK error, one per service purchase path. Each mirrors a fmt.Errorf in +// cloud-commitments-go providers/aws/services at the pinned version. +var awsLibraryHeads = []string{ + "failed to purchase EC2 RI: ", // ec2/client.go:185 + "failed to purchase RDS RI: ", // rds/client.go:194 + "failed to purchase Reserved Cache Node: ", // elasticache/client.go:195 + "failed to purchase OpenSearch RI: ", // opensearch/client.go:225 + "failed to purchase Redshift Reserved Node: ", // redshift/client.go:165 + "failed to purchase MemoryDB Reserved Nodes: ", // memorydb/client.go:196 + "failed to purchase Savings Plan: ", // savingsplans/client.go:272 +} + +// stripKnownHeads removes the executor's per-rec resource token, its +// "purchase failed: " wrapper and one library head, each optional and in that +// order. Any other text is left in place so the caller's match fails. +func stripKnownHeads(seg string) string { + seg = resourceHeadRE.ReplaceAllString(seg, "") + seg = strings.TrimPrefix(seg, executorPurchaseFailedHead) + for _, h := range awsLibraryHeads { + if strings.HasPrefix(seg, h) { + return seg[len(h):] + } + } + return seg +} + func definiteAWSSegment(seg string) bool { if strings.Contains(seg, "outcome unknown") { return false } + seg = stripKnownHeads(seg) m := definiteAWSRE.FindStringSubmatch(seg) return len(m) == 3 && definiteAWSCodes[m[2]] } @@ -231,7 +264,7 @@ func IsDefiniteAWSRefusal(recs []config.RecommendationRecord, stored string) boo return false } for i := range recs { - if recs[i].Provider != "" && recs[i].Provider != "aws" { + if recs[i].Provider != "aws" { return false } } diff --git a/internal/purchase/errsummary_provider_test.go b/internal/purchase/errsummary_provider_test.go index 70486e485..a62c2b892 100644 --- a/internal/purchase/errsummary_provider_test.go +++ b/internal/purchase/errsummary_provider_test.go @@ -145,6 +145,8 @@ func TestIsDefiniteAWSRefusal(t *testing.T) { aws := func(status, code string) string { return "operation error EC2: X, https response error StatusCode: " + status + ", RequestID: r, api error " + code + ": msg" } + storedEC2 := "m5.large: purchase failed: failed to purchase EC2 RI: operation error EC2: PurchaseReservedInstancesOffering, https response error StatusCode: 403, RequestID: 11111111-2222-3333-4444-555555555555, api error UnauthorizedOperation: You are not authorized to perform this operation (account 123456789012)" + storedRDS := "db.r6g.large: purchase failed: failed to purchase RDS RI: operation error RDS: PurchaseReservedDBInstancesOffering, https response error StatusCode: 403, RequestID: 11111111-2222-3333-4444-555555555555, api error AccessDenied: User is not authorized" azure429 := "PUT https://management.azure.com/x\nRESPONSE 429: 429 Too Many\nERROR CODE: TooManyRequests" gcp := "commitment creation failed: googleapi: Error 403: Rate Limit Exceeded, rateLimitExceeded" cases := []struct { @@ -154,7 +156,19 @@ func TestIsDefiniteAWSRefusal(t *testing.T) { want bool }{ {"aws 403 denied", []string{"aws"}, aws("403", "UnauthorizedOperation"), true}, - {"legacy empty provider", []string{""}, aws("403", "AccessDenied"), true}, + {"empty provider is not aws", []string{""}, aws("403", "AccessDenied"), false}, + {"empty provider beside aws", []string{"aws", ""}, aws("403", "AccessDenied"), false}, + {"ec2 stored shape", []string{"aws"}, storedEC2, true}, + {"rds stored shape", []string{"aws"}, storedRDS, true}, + {"stored shape without resource type", []string{"aws"}, "purchase failed: failed to purchase EC2 RI: " + aws("403", "AccessDenied"), true}, + {"stored shape without purchase failed", []string{"aws"}, "m5.large: failed to purchase EC2 RI: " + aws("403", "AccessDenied"), true}, + {"stored shape, two recs", []string{"aws", "aws"}, storedEC2 + "; " + storedRDS, true}, + {"stored shape, partial prefix", []string{"aws"}, partialRunPrefix + storedEC2, true}, + {"unknown head before operation error", []string{"aws"}, "m5.large: purchase failed: something else: " + aws("403", "AccessDenied"), false}, + {"unknown library head", []string{"aws"}, "m5.large: purchase failed: failed to purchase GPU RI: " + aws("403", "AccessDenied"), false}, + {"head after another head", []string{"aws"}, "purchase failed: purchase failed: " + aws("403", "AccessDenied"), false}, + {"stored shape 5xx", []string{"aws"}, strings.Replace(storedEC2, "StatusCode: 403", "StatusCode: 503", 1), false}, + {"stored shape outcome unknown", []string{"aws"}, "m5.large: purchase failed: purchase outcome unknown: " + aws("403", "AccessDenied"), false}, {"aws throttle 400", []string{"aws"}, aws("400", "Throttling"), true}, {"aws invalid offering", []string{"aws"}, aws("400", "InvalidParameterValue"), true}, {"per-rec segments", []string{"aws", "aws"}, "a: " + aws("403", "AccessDenied") + "; b: " + aws("400", "Throttling"), true}, From e9a6f804a0793d4c52349603c9ff2485185b3ef4 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 5 Nov 2026 00:56:42 +0100 Subject: [PATCH 2/4] fix(history): treat only single-attempt AWS purchase refusals as definite RDS, ElastiCache, OpenSearch, MemoryDB and Savings Plans purchase calls carry an idempotency token, so the provider keeps the SDK retries. A first attempt can buy and lose its response, and a later attempt can then store a 4xx, which the gate accepted as proof that nothing was bought. The duplicate-purchase hint is now dropped only for the EC2 and Redshift library heads, whose calls always make one attempt. Any text with an SDK retry-exhausted marker keeps the hint, a segment without one of those heads keeps it, and the status must be the one in the SDK's "https response error" part. Tests cover each retrying service with a plain and a retried 4xx, the retry-exhausted EC2 and Redshift shapes, a head in another case and a status outside the response error. Fixtures that used the bare SDK error now use the shape the executor stores. The cfn-lint hook was skipped locally: cfn-lint 1.58.0 does not install on Python 3.15, and this change touches no CloudFormation template. Co-Authored-By: Claude Opus 5.5 --- internal/api/history_duplicate_hint_test.go | 13 ++- internal/api/purchase_failure_notes_test.go | 2 +- internal/purchase/errsummary.go | 54 +++++++------ internal/purchase/errsummary_provider_test.go | 79 ++++++++++++------- 4 files changed, 93 insertions(+), 55 deletions(-) diff --git a/internal/api/history_duplicate_hint_test.go b/internal/api/history_duplicate_hint_test.go index d625a6a03..57c59823d 100644 --- a/internal/api/history_duplicate_hint_test.go +++ b/internal/api/history_duplicate_hint_test.go @@ -8,9 +8,12 @@ import ( "github.com/stretchr/testify/assert" ) +// ec2Head is the single-attempt EC2 library head the executor stores before the SDK error. +const ec2Head = "failed to purchase EC2 RI: " + func TestHistoryRow_DuplicateGuardHint(t *testing.T) { ec2 := []config.RecommendationRecord{{Provider: "aws", Service: "ec2"}} - denied := executionToHistoryRow(config.PurchaseExecution{Status: "failed", Error: fakeAWSDenied, Recommendations: ec2}, "", "") + denied := executionToHistoryRow(config.PurchaseExecution{Status: "failed", Error: ec2Head + fakeAWSDenied, Recommendations: ec2}, "", "") assert.False(t, strings.Contains(denied.OpsHint, "duplicate guard"), "definite refusal hides the warning") unknown := executionToHistoryRow(config.PurchaseExecution{Status: "failed", Error: "purchase outcome unknown", Recommendations: ec2}, "", "") assert.Contains(t, unknown.OpsHint, "duplicate guard") @@ -19,7 +22,7 @@ func TestHistoryRow_DuplicateGuardHint(t *testing.T) { // One refused rec does not prove that another, whose outcome is unknown, bought nothing. mixed := executionToHistoryRow(config.PurchaseExecution{Status: "failed", Error: "t4g.nano: purchase outcome unknown; t4g.micro: " + fakeAWSDenied, Recommendations: ec2}, "", "") assert.Contains(t, mixed.OpsHint, "duplicate guard", "a mixed failure keeps the warning") - allDenied := executionToHistoryRow(config.PurchaseExecution{Status: "failed", Error: "t4g.nano: " + fakeAWSDenied + "; t4g.micro: " + fakeAWSDenied, Recommendations: ec2}, "", "") + allDenied := executionToHistoryRow(config.PurchaseExecution{Status: "failed", Error: "t4g.nano: " + ec2Head + fakeAWSDenied + "; t4g.micro: " + ec2Head + fakeAWSDenied, Recommendations: ec2}, "", "") assert.NotContains(t, allDenied.OpsHint, "duplicate guard", "every rec refused hides the warning") } @@ -54,10 +57,12 @@ func TestHistoryRow_DuplicateHintKeptOutsideDefiniteAWSRefusals(t *testing.T) { func TestHistoryRow_ExecutorStoredAWSRefusalDropsDuplicateHint(t *testing.T) { const ec2Stored = "m5.large: purchase failed: failed to purchase EC2 RI: operation error EC2: PurchaseReservedInstancesOffering, https response error StatusCode: 403, RequestID: 11111111-2222-3333-4444-555555555555, api error UnauthorizedOperation: You are not authorized (account 123456789012)" + const redshiftStored = "ra3.xlplus: purchase failed: failed to purchase Redshift Reserved Node: operation error Redshift: PurchaseReservedNodeOffering, https response error StatusCode: 403, RequestID: 11111111-2222-3333-4444-555555555555, api error AccessDenied: denied" + // RDS calls carry an idempotency token and keep the SDK retries, so its 4xx proves nothing. const rdsStored = "db.r6g.large: purchase failed: failed to purchase RDS RI: operation error RDS: PurchaseReservedDBInstancesOffering, https response error StatusCode: 403, RequestID: 11111111-2222-3333-4444-555555555555, api error AccessDenied: denied" ec2 := config.RecommendationRecord{Provider: "aws", Service: "ec2"} redshift := config.RecommendationRecord{Provider: "aws", Service: "redshift"} - for name, stored := range map[string]string{"ec2": ec2Stored, "rds beside ec2": ec2Stored + "; " + rdsStored} { + for name, stored := range map[string]string{"ec2": ec2Stored, "redshift beside ec2": ec2Stored + "; " + redshiftStored} { row := executionToHistoryRow(config.PurchaseExecution{Status: "failed", Error: stored, Recommendations: []config.RecommendationRecord{ec2, redshift}}, "", "") assert.NotContains(t, row.OpsHint, "duplicate guard", name) assert.NotEmpty(t, row.OpsHint, name+": ops_hint stays set so Retry stays hidden") @@ -71,6 +76,8 @@ func TestHistoryRow_ExecutorStoredAWSRefusalDropsDuplicateHint(t *testing.T) { "5xx": {[]config.RecommendationRecord{ec2}, strings.Replace(ec2Stored, "StatusCode: 403", "StatusCode: 503", 1)}, "empty provider": {[]config.RecommendationRecord{{Service: "ec2"}}, fakeAWSDenied}, "mixed run": {[]config.RecommendationRecord{ec2}, ec2Stored + "; t4g.nano: purchase failed: purchase outcome unknown"}, + "rds beside ec2": {[]config.RecommendationRecord{ec2, {Provider: "aws", Service: "rds"}}, ec2Stored + "; " + rdsStored}, + "retried ec2": {[]config.RecommendationRecord{ec2}, strings.Replace(ec2Stored, ", https response error", ", exceeded maximum number of attempts, 3, https response error", 1)}, } { row := executionToHistoryRow(config.PurchaseExecution{Status: "failed", Error: c.stored, Recommendations: c.recs}, "", "") assert.Contains(t, row.OpsHint, "duplicate guard", name) diff --git a/internal/api/purchase_failure_notes_test.go b/internal/api/purchase_failure_notes_test.go index 38d0d83e7..9102db0ef 100644 --- a/internal/api/purchase_failure_notes_test.go +++ b/internal/api/purchase_failure_notes_test.go @@ -83,7 +83,7 @@ func TestTokenActionError_StalePricingShowsOnlySentinelText(t *testing.T) { // A refused AWS purchase keeps ops_hint (the History UI hides Retry on it) but // carries no duplicate-purchase warning. func TestHistoryRow_DefiniteAWSRefusalKeepsOpsHintWithoutDuplicateWarning(t *testing.T) { - denied := "operation error EC2: X, https response error StatusCode: 403, RequestID: r, api error UnauthorizedOperation: msg" + denied := "m5.large: purchase failed: failed to purchase EC2 RI: operation error EC2: X, https response error StatusCode: 403, RequestID: r, api error UnauthorizedOperation: msg" row := executionToHistoryRow(config.PurchaseExecution{ ExecutionID: "e-1", Status: "failed", Error: denied, Recommendations: []config.RecommendationRecord{{Provider: "aws", Service: "ec2"}}, diff --git a/internal/purchase/errsummary.go b/internal/purchase/errsummary.go index 9ff591bc2..e1af4b4d4 100644 --- a/internal/purchase/errsummary.go +++ b/internal/purchase/errsummary.go @@ -155,7 +155,7 @@ var ( gcpReasonRE = regexp.MustCompile(`Reason: ([A-Za-z]+), Message:`) gcpErrorRE = regexp.MustCompile(`googleapi: Error (\d{3}): [^\n]*?(?:, ([A-Za-z]+))?(?:\n|$)`) resourceHeadRE = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._-]*: `) - definiteAWSRE = regexp.MustCompile(`^operation error [^\n]*StatusCode: (4\d{2}),[^\n]*api error ([A-Za-z0-9_.]+): [^\n]*$`) + definiteAWSRE = regexp.MustCompile(`^operation error [^\n]*, https response error StatusCode: (4\d{2}),[^\n]*api error ([A-Za-z0-9_.]+): [^\n]*$`) ) func statusOf(re *regexp.Regexp, s string) int { @@ -219,45 +219,55 @@ var definiteAWSCodes = map[string]bool{ // error (internal/purchase/execution.go, "purchase failed: %w"). const executorPurchaseFailedHead = "purchase failed: " -// awsLibraryHeads are the exact heads the AWS provider library puts in front of -// the SDK error, one per service purchase path. Each mirrors a fmt.Errorf in -// cloud-commitments-go providers/aws/services at the pinned version. -var awsLibraryHeads = []string{ - "failed to purchase EC2 RI: ", // ec2/client.go:185 - "failed to purchase RDS RI: ", // rds/client.go:194 - "failed to purchase Reserved Cache Node: ", // elasticache/client.go:195 - "failed to purchase OpenSearch RI: ", // opensearch/client.go:225 - "failed to purchase Redshift Reserved Node: ", // redshift/client.go:165 - "failed to purchase MemoryDB Reserved Nodes: ", // memorydb/client.go:196 - "failed to purchase Savings Plan: ", // savingsplans/client.go:272 +// singleAttemptAWSHeads are the exact AWS provider library heads of the +// purchase calls that always make one attempt (RetryMaxAttempts = 1), so a +// stored 4xx proves the only request was refused. Each mirrors a fmt.Errorf in +// cloud-commitments-go providers/aws/services at the pinned version. RDS, +// ElastiCache, OpenSearch, MemoryDB and Savings Plans calls carry an +// idempotency token and keep the SDK retries (purchasecfg.SingleAttemptIf), so +// a later 4xx can follow a lost first response that did buy: never listed. +var singleAttemptAWSHeads = []string{ + "failed to purchase EC2 RI: ", // ec2/client.go:185 + "failed to purchase Redshift Reserved Node: ", // redshift/client.go:165 } -// stripKnownHeads removes the executor's per-rec resource token, its -// "purchase failed: " wrapper and one library head, each optional and in that -// order. Any other text is left in place so the caller's match fails. -func stripKnownHeads(seg string) string { +// retryExhaustedMarkers are the SDK texts of a call that made several attempts. +var retryExhaustedMarkers = []string{"exceeded maximum number of attempts", "retry quota exceeded"} + +// singleAttemptSDKError removes the executor's optional per-rec resource token +// and "purchase failed: " wrapper, then returns the SDK error after a required +// single-attempt library head. ok is false for any other head or none. +func singleAttemptSDKError(seg string) (string, bool) { seg = resourceHeadRE.ReplaceAllString(seg, "") seg = strings.TrimPrefix(seg, executorPurchaseFailedHead) - for _, h := range awsLibraryHeads { + for _, h := range singleAttemptAWSHeads { if strings.HasPrefix(seg, h) { - return seg[len(h):] + return seg[len(h):], true } } - return seg + return "", false } func definiteAWSSegment(seg string) bool { if strings.Contains(seg, "outcome unknown") { return false } - seg = stripKnownHeads(seg) - m := definiteAWSRE.FindStringSubmatch(seg) + for _, m := range retryExhaustedMarkers { + if strings.Contains(seg, m) { + return false + } + } + sdkErr, ok := singleAttemptSDKError(seg) + if !ok { + return false + } + m := definiteAWSRE.FindStringSubmatch(sdkErr) return len(m) == 3 && definiteAWSCodes[m[2]] } // IsDefiniteAWSRefusal reports whether a stored error proves nothing was // bought: every rec is AWS and every "; "-separated segment is an AWS 4xx -// with a listed pre-send code. Azure, GCP, 5xx, a missing code or a server +// with a listed pre-send code from a single-attempt purchase call. Azure, GCP, 5xx, a missing code or a server // note anywhere means the outcome is not proven, so the answer is false. func IsDefiniteAWSRefusal(recs []config.RecommendationRecord, stored string) bool { if len(recs) == 0 { diff --git a/internal/purchase/errsummary_provider_test.go b/internal/purchase/errsummary_provider_test.go index a62c2b892..b1123e984 100644 --- a/internal/purchase/errsummary_provider_test.go +++ b/internal/purchase/errsummary_provider_test.go @@ -142,50 +142,71 @@ func TestSanitizeStoredError_WholeRemainderReplaced(t *testing.T) { } func TestIsDefiniteAWSRefusal(t *testing.T) { - aws := func(status, code string) string { - return "operation error EC2: X, https response error StatusCode: " + status + ", RequestID: r, api error " + code + ": msg" + sdk := func(svc, status, code string) string { + return "operation error " + svc + ": X, https response error StatusCode: " + status + ", RequestID: r, api error " + code + ": msg" } + retried := func(svc, status, code string) string { + return "operation error " + svc + ": X, exceeded maximum number of attempts, 3, https response error StatusCode: " + status + ", RequestID: r, api error " + code + ": msg" + } + ec2 := func(status, code string) string { return "failed to purchase EC2 RI: " + sdk("EC2", status, code) } storedEC2 := "m5.large: purchase failed: failed to purchase EC2 RI: operation error EC2: PurchaseReservedInstancesOffering, https response error StatusCode: 403, RequestID: 11111111-2222-3333-4444-555555555555, api error UnauthorizedOperation: You are not authorized to perform this operation (account 123456789012)" - storedRDS := "db.r6g.large: purchase failed: failed to purchase RDS RI: operation error RDS: PurchaseReservedDBInstancesOffering, https response error StatusCode: 403, RequestID: 11111111-2222-3333-4444-555555555555, api error AccessDenied: User is not authorized" + storedRedshift := "ra3.xlplus: purchase failed: failed to purchase Redshift Reserved Node: " + sdk("Redshift", "400", "InvalidParameterValue") azure429 := "PUT https://management.azure.com/x\nRESPONSE 429: 429 Too Many\nERROR CODE: TooManyRequests" gcp := "commitment creation failed: googleapi: Error 403: Rate Limit Exceeded, rateLimitExceeded" - cases := []struct { + type tc struct { name string recs []string err string want bool - }{ - {"aws 403 denied", []string{"aws"}, aws("403", "UnauthorizedOperation"), true}, - {"empty provider is not aws", []string{""}, aws("403", "AccessDenied"), false}, - {"empty provider beside aws", []string{"aws", ""}, aws("403", "AccessDenied"), false}, + } + cases := []tc{ + {"ec2 403 denied", []string{"aws"}, ec2("403", "UnauthorizedOperation"), true}, + {"empty provider is not aws", []string{""}, ec2("403", "AccessDenied"), false}, + {"empty provider beside aws", []string{"aws", ""}, ec2("403", "AccessDenied"), false}, {"ec2 stored shape", []string{"aws"}, storedEC2, true}, - {"rds stored shape", []string{"aws"}, storedRDS, true}, - {"stored shape without resource type", []string{"aws"}, "purchase failed: failed to purchase EC2 RI: " + aws("403", "AccessDenied"), true}, - {"stored shape without purchase failed", []string{"aws"}, "m5.large: failed to purchase EC2 RI: " + aws("403", "AccessDenied"), true}, - {"stored shape, two recs", []string{"aws", "aws"}, storedEC2 + "; " + storedRDS, true}, + {"redshift stored shape", []string{"aws"}, storedRedshift, true}, + {"stored shape without resource type", []string{"aws"}, "purchase failed: " + ec2("403", "AccessDenied"), true}, + {"stored shape without purchase failed", []string{"aws"}, "m5.large: " + ec2("403", "AccessDenied"), true}, + {"stored shape, two recs", []string{"aws", "aws"}, storedEC2 + "; " + storedRedshift, true}, {"stored shape, partial prefix", []string{"aws"}, partialRunPrefix + storedEC2, true}, - {"unknown head before operation error", []string{"aws"}, "m5.large: purchase failed: something else: " + aws("403", "AccessDenied"), false}, - {"unknown library head", []string{"aws"}, "m5.large: purchase failed: failed to purchase GPU RI: " + aws("403", "AccessDenied"), false}, - {"head after another head", []string{"aws"}, "purchase failed: purchase failed: " + aws("403", "AccessDenied"), false}, + {"bare sdk error, no library head", []string{"aws"}, sdk("EC2", "403", "AccessDenied"), false}, + {"resource token, no library head", []string{"aws"}, "m5.large: " + sdk("EC2", "403", "AccessDenied"), false}, + {"unknown head before operation error", []string{"aws"}, "m5.large: purchase failed: something else: " + sdk("EC2", "403", "AccessDenied"), false}, + {"unknown library head", []string{"aws"}, "m5.large: purchase failed: failed to purchase GPU RI: " + sdk("EC2", "403", "AccessDenied"), false}, + {"head in another case", []string{"aws"}, "Failed to purchase EC2 RI: " + sdk("EC2", "403", "AccessDenied"), false}, + {"head after another head", []string{"aws"}, "purchase failed: purchase failed: " + ec2("403", "AccessDenied"), false}, {"stored shape 5xx", []string{"aws"}, strings.Replace(storedEC2, "StatusCode: 403", "StatusCode: 503", 1), false}, - {"stored shape outcome unknown", []string{"aws"}, "m5.large: purchase failed: purchase outcome unknown: " + aws("403", "AccessDenied"), false}, - {"aws throttle 400", []string{"aws"}, aws("400", "Throttling"), true}, - {"aws invalid offering", []string{"aws"}, aws("400", "InvalidParameterValue"), true}, - {"per-rec segments", []string{"aws", "aws"}, "a: " + aws("403", "AccessDenied") + "; b: " + aws("400", "Throttling"), true}, - {"partial prefix", []string{"aws"}, partialRunPrefix + "a: " + aws("403", "AccessDenied"), true}, - {"5xx", []string{"aws"}, aws("503", "AccessDenied"), false}, - {"no code", []string{"aws"}, "operation error EC2: X, https response error StatusCode: 403", false}, - {"unlisted code", []string{"aws"}, aws("400", "SomethingOdd"), false}, + {"stored shape outcome unknown", []string{"aws"}, "m5.large: purchase failed: purchase outcome unknown: " + ec2("403", "AccessDenied"), false}, + {"status outside the response error", []string{"aws"}, "failed to purchase EC2 RI: operation error EC2: X, https request send failed: StatusCode: 403, api error AccessDenied: msg", false}, + {"ec2 throttle 400", []string{"aws"}, ec2("400", "Throttling"), true}, + {"ec2 invalid offering", []string{"aws"}, ec2("400", "InvalidParameterValue"), true}, + {"per-rec segments", []string{"aws", "aws"}, "a: " + ec2("403", "AccessDenied") + "; b: " + ec2("400", "Throttling"), true}, + {"ec2 retry exhausted", []string{"aws"}, "failed to purchase EC2 RI: " + retried("EC2", "400", "Throttling"), false}, + {"redshift retry exhausted", []string{"aws"}, "failed to purchase Redshift Reserved Node: " + retried("Redshift", "400", "Throttling"), false}, + {"retry quota exceeded", []string{"aws"}, "failed to purchase EC2 RI: operation error EC2: X, failed to get rate limit token, retry quota exceeded, https response error StatusCode: 400, api error Throttling: msg", false}, + {"5xx", []string{"aws"}, ec2("503", "AccessDenied"), false}, + {"no code", []string{"aws"}, "failed to purchase EC2 RI: operation error EC2: X, https response error StatusCode: 403", false}, + {"unlisted code", []string{"aws"}, ec2("400", "SomethingOdd"), false}, {"azure rec", []string{"azure"}, azure429, false}, {"azure text on aws rec", []string{"aws"}, azure429, false}, {"gcp rec", []string{"gcp"}, gcp, false}, - {"mixed recs", []string{"aws", "gcp"}, aws("403", "AccessDenied"), false}, + {"mixed recs", []string{"aws", "gcp"}, ec2("403", "AccessDenied"), false}, {"server note", []string{"aws"}, "history_write_failed: commitment c purchased but its history record failed to save", false}, - {"unknown outcome wrapper", []string{"aws"}, "purchase outcome unknown: the request may have been sent; reconcile before retrying: " + aws("400", "Throttling"), false}, - {"one unknown segment", []string{"aws", "aws"}, "a: " + aws("403", "AccessDenied") + "; b: purchase outcome unknown", false}, - {"semicolon in message", []string{"aws"}, aws("400", "InvalidParameterValue") + "; reserved elsewhere", false}, - {"no recs", nil, aws("403", "AccessDenied"), false}, - {"empty", []string{"aws"}, "", false}, + {"unknown outcome wrapper", []string{"aws"}, "purchase outcome unknown: the request may have been sent; reconcile before retrying: " + ec2("400", "Throttling"), false}, + } + // Token-carrying services keep the SDK retries, so neither a single-attempt + // looking 4xx nor a retry-exhausted one proves nothing was bought. + for _, r := range []struct{ head, svc string }{ + {"failed to purchase RDS RI: ", "RDS"}, + {"failed to purchase Reserved Cache Node: ", "ElastiCache"}, + {"failed to purchase OpenSearch RI: ", "OpenSearch"}, + {"failed to purchase MemoryDB Reserved Nodes: ", "MemoryDB"}, + {"failed to purchase Savings Plan: ", "savingsplans"}, + } { + cases = append(cases, + tc{r.svc + " 4xx keeps the hint", []string{"aws"}, "x.large: purchase failed: " + r.head + sdk(r.svc, "403", "AccessDenied"), false}, + tc{r.svc + " retried 4xx keeps the hint", []string{"aws"}, "x.large: purchase failed: " + r.head + retried(r.svc, "400", "ThrottlingException"), false}, + ) } for _, c := range cases { var recs []config.RecommendationRecord From 3765a0a22fd97c3ce0fd0a6787cdfc8c0ac05cd4 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 5 Nov 2026 01:33:55 +0100 Subject: [PATCH 3/4] test(history): restore the dropped definite-refusal cases The previous commit's table rewrite dropped five TestIsDefiniteAWSRefusal cases: no recs (the only test of the empty-recs guard), an empty error, a semicolon inside a message, one unknown segment beside a refusal, and a plain partial-run prefix. They are back, built on the EC2 head. The cfn-lint hook was skipped locally: cfn-lint 1.58.0 does not install on Python 3.15, and this change touches no CloudFormation template. Co-Authored-By: Claude Opus 5.5 --- internal/purchase/errsummary_provider_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/purchase/errsummary_provider_test.go b/internal/purchase/errsummary_provider_test.go index b1123e984..c52d50dd3 100644 --- a/internal/purchase/errsummary_provider_test.go +++ b/internal/purchase/errsummary_provider_test.go @@ -193,6 +193,11 @@ func TestIsDefiniteAWSRefusal(t *testing.T) { {"mixed recs", []string{"aws", "gcp"}, ec2("403", "AccessDenied"), false}, {"server note", []string{"aws"}, "history_write_failed: commitment c purchased but its history record failed to save", false}, {"unknown outcome wrapper", []string{"aws"}, "purchase outcome unknown: the request may have been sent; reconcile before retrying: " + ec2("400", "Throttling"), false}, + {"partial prefix", []string{"aws"}, partialRunPrefix + "a: " + ec2("403", "AccessDenied"), true}, + {"one unknown segment", []string{"aws", "aws"}, "a: " + ec2("403", "AccessDenied") + "; b: purchase outcome unknown", false}, + {"semicolon in message", []string{"aws"}, ec2("400", "InvalidParameterValue") + "; reserved elsewhere", false}, + {"no recs", nil, ec2("403", "AccessDenied"), false}, + {"empty", []string{"aws"}, "", false}, } // Token-carrying services keep the SDK retries, so neither a single-attempt // looking 4xx nor a retry-exhausted one proves nothing was bought. From 1354e6446eaa950939d3d74f0ef3cf55485948d8 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 5 Nov 2026 12:51:39 +0100 Subject: [PATCH 4/4] test(history): preallocate the definite-refusal case table CI's golangci-lint prealloc check flagged the case slice that grows in the retrying-service loop. The table now has its capacity up front, and the retrying services are a named list. The cfn-lint hook was skipped locally: cfn-lint 1.58.0 does not install on Python 3.15, and this change touches no CloudFormation template. Co-Authored-By: Claude Opus 5.5 --- internal/purchase/errsummary_provider_test.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/internal/purchase/errsummary_provider_test.go b/internal/purchase/errsummary_provider_test.go index c52d50dd3..3065a2bbf 100644 --- a/internal/purchase/errsummary_provider_test.go +++ b/internal/purchase/errsummary_provider_test.go @@ -159,7 +159,14 @@ func TestIsDefiniteAWSRefusal(t *testing.T) { err string want bool } - cases := []tc{ + retrying := []struct{ head, svc string }{ + {"failed to purchase RDS RI: ", "RDS"}, + {"failed to purchase Reserved Cache Node: ", "ElastiCache"}, + {"failed to purchase OpenSearch RI: ", "OpenSearch"}, + {"failed to purchase MemoryDB Reserved Nodes: ", "MemoryDB"}, + {"failed to purchase Savings Plan: ", "savingsplans"}, + } + base := []tc{ {"ec2 403 denied", []string{"aws"}, ec2("403", "UnauthorizedOperation"), true}, {"empty provider is not aws", []string{""}, ec2("403", "AccessDenied"), false}, {"empty provider beside aws", []string{"aws", ""}, ec2("403", "AccessDenied"), false}, @@ -199,15 +206,11 @@ func TestIsDefiniteAWSRefusal(t *testing.T) { {"no recs", nil, ec2("403", "AccessDenied"), false}, {"empty", []string{"aws"}, "", false}, } + cases := make([]tc, 0, len(base)+2*len(retrying)) + cases = append(cases, base...) // Token-carrying services keep the SDK retries, so neither a single-attempt // looking 4xx nor a retry-exhausted one proves nothing was bought. - for _, r := range []struct{ head, svc string }{ - {"failed to purchase RDS RI: ", "RDS"}, - {"failed to purchase Reserved Cache Node: ", "ElastiCache"}, - {"failed to purchase OpenSearch RI: ", "OpenSearch"}, - {"failed to purchase MemoryDB Reserved Nodes: ", "MemoryDB"}, - {"failed to purchase Savings Plan: ", "savingsplans"}, - } { + for _, r := range retrying { cases = append(cases, tc{r.svc + " 4xx keeps the hint", []string{"aws"}, "x.large: purchase failed: " + r.head + sdk(r.svc, "403", "AccessDenied"), false}, tc{r.svc + " retried 4xx keeps the hint", []string{"aws"}, "x.large: purchase failed: " + r.head + retried(r.svc, "400", "ThrottlingException"), false},