From 482e78a9dde903fd56f0bb03bcf1fd039987b7ee Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 4 Nov 2026 08:37:16 +0100 Subject: [PATCH 1/4] chore(deps): bump cloud-commitments-go modules to 39dc6e7 Picks up common.ErrRefusedBeforePurchase and the AWS approved-upfront guards that wrap it, plus the Azure Advisor pricing changes. Co-Authored-By: Claude Opus 5.5 --- go.mod | 8 ++++---- go.sum | 16 ++++++++-------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/go.mod b/go.mod index 7f5f36f..942cb16 100644 --- a/go.mod +++ b/go.mod @@ -73,10 +73,10 @@ require ( ) require ( - github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261028031643-8785de4553bb - github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261028031643-8785de4553bb - github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261028031643-8785de4553bb - github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261028031643-8785de4553bb + github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261104073158-39dc6e76ed43 + github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261104073158-39dc6e76ed43 + github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261104073158-39dc6e76ed43 + github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261104073158-39dc6e76ed43 github.com/google/jsonschema-go v0.4.3 github.com/google/uuid v1.6.0 github.com/modelcontextprotocol/go-sdk v1.8.0 diff --git a/go.sum b/go.sum index 2306451..8804aa5 100644 --- a/go.sum +++ b/go.sum @@ -74,14 +74,14 @@ github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/cloudmock v0 github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/cloudmock v0.57.0/go.mod h1:dzcEjy1WJ0Q4u9twNR3LcLhNoYMRCrMCMafpxa0TjPQ= github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.57.0 h1:RoO5+d7uCmDqovLrHCr2/BuViUXvdcrNxyNM1pN9dDQ= github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.57.0/go.mod h1:YqwkQPrWSC7+byyc1VlKbWLBF5JsW5IoL6xUkemYSXk= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261028031643-8785de4553bb h1:mbXfKznDVrwnwiVCV2W0ncDJL026DQ3Ohge2++decJk= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261028031643-8785de4553bb/go.mod h1:pseCFRgm1rzBKomktjw6E0ayxmVE7XfHHiOmew4ka+g= -github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261028031643-8785de4553bb h1:soLuvYTccFmODZSOZ5lEf2FltCW8U+LTwQ4TsCBpgUE= -github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261028031643-8785de4553bb/go.mod h1:NjNxaaNeYU3CTAcWUCOxaN+NeY7aveSyI/jNXEKz7/c= -github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261028031643-8785de4553bb h1:jOkmjkGYwF5jXEru9/rVLSTMuKH3QiRpKM0pEHqEAeE= -github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261028031643-8785de4553bb/go.mod h1:X+73EdStvYV6pqEVZZFyMfLG1frORlO9B1IsuK0LfOs= -github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261028031643-8785de4553bb h1:bbgtDaS1H0bs9H+QC2YtMle1WRV9sWadUPgXDYIYqVc= -github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261028031643-8785de4553bb/go.mod h1:aj4d0YEmBg1A46rIXKG+GkTFc1d/xwmveJ97kDD1oPs= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261104073158-39dc6e76ed43 h1:XSrxaJ8RmRaNyqFTjI9LMFTNjEUkz85Bw+8eUDyqmn0= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261104073158-39dc6e76ed43/go.mod h1:pseCFRgm1rzBKomktjw6E0ayxmVE7XfHHiOmew4ka+g= +github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261104073158-39dc6e76ed43 h1:4mo8ao/sqPQgu00DtvfosogAKDes5F6gU3RHLkQQWoI= +github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261104073158-39dc6e76ed43/go.mod h1:6qNUs6GcB4Dn/mOEbd7Y+IG4tWzyV7Rtc/z0YwC2nwE= +github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261104073158-39dc6e76ed43 h1:A5KbgXxnZ9psdreid61aH5PbXiIKUvyR/WN+st6Lo24= +github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261104073158-39dc6e76ed43/go.mod h1:X+73EdStvYV6pqEVZZFyMfLG1frORlO9B1IsuK0LfOs= +github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261104073158-39dc6e76ed43 h1:+T6roXAS0IsEJoLhpG9OVxoMk5zXRzZB5ZVOmQo092o= +github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261104073158-39dc6e76ed43/go.mod h1:aj4d0YEmBg1A46rIXKG+GkTFc1d/xwmveJ97kDD1oPs= github.com/aws/aws-sdk-go-v2 v1.47.4 h1:bTvk6UdSSSvaQsfLVj1JRDEEqj/PDRwGf1KanaRzau0= github.com/aws/aws-sdk-go-v2 v1.47.4/go.mod h1:XkrjAZvr705Nytv6PkPUhoSB2W9+vVHBp5oAWpkh8Ak= github.com/aws/aws-sdk-go-v2/config v1.33.10 h1:RRb0GghYqq2ikMXzfT4r9NO9lVhOld3FIhJJWDIdk+w= From fa399c69c953225e32365613c058f0b22cb8c00d Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 4 Nov 2026 08:43:57 +0100 Subject: [PATCH 2/4] fix(budget): release the reservation when the provider refuses before purchase purchaseSpent now treats a failure as unspent when every error wraps common.ErrRefusedBeforePurchase, none wraps ErrOutcomeUnknown, and the result has no success or commitment ID. Other errors stay counted. Closes #124 Co-Authored-By: Claude Opus 5.5 --- tools/budget.go | 28 +++++++++++++++++---- tools/budget_persist_test.go | 48 ++++++++++++++++++++++++++++++++++++ tools/budget_test.go | 32 ++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 5 deletions(-) diff --git a/tools/budget.go b/tools/budget.go index f71f1ae..e67da1c 100644 --- a/tools/budget.go +++ b/tools/budget.go @@ -1,6 +1,7 @@ package tools import ( + "errors" "fmt" "os" "strings" @@ -86,16 +87,33 @@ func (l *budgetLedger) committedUSD() float64 { } // purchaseSpent reports whether a purchase that reached PurchaseCommitment -// must stay counted. Only a clean no-op (nil error, no success, no error) or -// an adopted existing commitment returned with no error is released; every -// error (err or result.Error), including one returned beside ExistingCommitment, and any other -// result may have spent money, so it stays counted. +// must stay counted. A clean no-op (nil error, no success, no error) or an +// adopted existing commitment returned with no error is released. A failure is +// released only when every error carries common.ErrRefusedBeforePurchase, none +// carries ErrOutcomeUnknown, and the result shows no success or commitment ID: +// the provider refused before any purchase call. Any other error, including +// one returned beside ExistingCommitment, may have spent money and stays counted. func purchaseSpent(result common.PurchaseResult, err error) bool { if err != nil || result.Error != nil { - return true + return !refusedBeforePurchase(result, err) } if result.ExistingCommitment { return false } return result.Success } + +func refusedBeforePurchase(result common.PurchaseResult, err error) bool { + if result.Success || result.CommitmentID != "" { + return false + } + for _, e := range []error{err, result.Error} { + if e == nil { + continue + } + if !errors.Is(e, common.ErrRefusedBeforePurchase) || errors.Is(e, common.ErrOutcomeUnknown) { + return false + } + } + return true +} diff --git a/tools/budget_persist_test.go b/tools/budget_persist_test.go index 8ce4ed9..aecee5e 100644 --- a/tools/budget_persist_test.go +++ b/tools/budget_persist_test.go @@ -3,11 +3,14 @@ package tools import ( + "context" + "errors" "os" "path/filepath" "testing" "time" + sdkec2 "github.com/aws/aws-sdk-go-v2/service/ec2" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -84,3 +87,48 @@ func TestUnsetBudgetStateStaysInMemory(t *testing.T) { require.NoError(t, err) assert.Empty(t, entries, "no state file may be created when %s is unset", EnvBudgetState) } + +// TestPersistedBudgetReleasedWhenUpfrontGuardRefuses runs the real EC2 client +// behind its API fake: the go#611 guard refuses before any purchase call, so +// nothing may stay committed. +func TestPersistedBudgetReleasedWhenUpfrontGuardRefuses(t *testing.T) { + path := persistedBudget(t) + api := newPricedEC2API(PaymentOptionAllUpfront, 500) + tool, _ := ec2ToolOverFakeAPI(api) + args := upfrontEC2Args(PaymentOptionAllUpfront, "budget-refused") + _, preview, err := tool.handle(context.Background(), nil, args) + require.NoError(t, err) + + api.fixedPrice = 526 // +5.2% over the approved 1500 + t.Setenv(EnvBudgetUSD, "100000") + _, _, err = tool.handle(context.Background(), nil, confirmed(args, preview.ApprovedUpfront, preview.ApprovedCurrency)) + require.ErrorContains(t, err, "exceeds the approved upfront") + assert.Zero(t, api.purchaseCalls) + st := readState(t, path) + assert.Zero(t, st.SpentUSD) + assert.Empty(t, st.Reservations) +} + +// TestPersistedBudgetKeepsCountedWhenPurchaseAPIFailsAfterGuard: the guard +// passes and the purchase call itself fails, so money may have moved. +func TestPersistedBudgetKeepsCountedWhenPurchaseAPIFailsAfterGuard(t *testing.T) { + path := persistedBudget(t) + t.Setenv(EnvBudgetUSD, "100000") + api := &failingPurchaseEC2API{pricedEC2API: newPricedEC2API(PaymentOptionAllUpfront, 500)} + tool, _ := ec2ToolOverFakeAPI(api) + args := upfrontEC2Args(PaymentOptionAllUpfront, "budget-api-fail") + _, preview, err := tool.handle(context.Background(), nil, args) + require.NoError(t, err) + + _, _, err = tool.handle(context.Background(), nil, confirmed(args, preview.ApprovedUpfront, preview.ApprovedCurrency)) + require.Error(t, err) + assert.Equal(t, 1, api.purchaseCalls) + assert.Greater(t, readState(t, path).SpentUSD, 0.0) +} + +type failingPurchaseEC2API struct{ *pricedEC2API } + +func (f *failingPurchaseEC2API) PurchaseReservedInstancesOffering(context.Context, *sdkec2.PurchaseReservedInstancesOfferingInput, ...func(*sdkec2.Options)) (*sdkec2.PurchaseReservedInstancesOfferingOutput, error) { + f.purchaseCalls++ + return nil, errors.New("api down") +} diff --git a/tools/budget_test.go b/tools/budget_test.go index 71ab036..c0f8ca4 100644 --- a/tools/budget_test.go +++ b/tools/budget_test.go @@ -3,6 +3,7 @@ package tools import ( "context" "errors" + "fmt" "sync" "sync/atomic" "testing" @@ -159,3 +160,34 @@ func TestBudgetEqualToLimitAllowed(t *testing.T) { require.ErrorContains(t, runWithNonce(fake, "c"), EnvBudgetUSD) assert.Equal(t, 2, fake.purchaseCalls) } + +func TestPurchaseSpent(t *testing.T) { + t.Parallel() + refused := fmt.Errorf("guard: %w", common.ErrRefusedBeforePurchase) + unknown := fmt.Errorf("%w: %w", common.ErrRefusedBeforePurchase, common.ErrOutcomeUnknown) + cases := []struct { + err error + name string + result common.PurchaseResult + spent bool + }{ + {refused, "sentinel in err only", common.PurchaseResult{}, false}, + {nil, "sentinel in result.Error only", common.PurchaseResult{Error: refused}, false}, + {refused, "sentinel in both", common.PurchaseResult{Error: refused}, false}, + {refused, "sentinel in err, plain result.Error", common.PurchaseResult{Error: assert.AnError}, true}, + {assert.AnError, "sentinel in result.Error, plain err", common.PurchaseResult{Error: refused}, true}, + {unknown, "sentinel with outcome unknown", common.PurchaseResult{}, true}, + {refused, "sentinel with success", common.PurchaseResult{Success: true}, true}, + {refused, "sentinel with commitment id", common.PurchaseResult{CommitmentID: "ri-1"}, true}, + {assert.AnError, "plain error", common.PurchaseResult{}, true}, + {nil, "nil error with success", common.PurchaseResult{Success: true}, true}, + {nil, "existing commitment", common.PurchaseResult{ExistingCommitment: true}, false}, + {nil, "clean no-op", common.PurchaseResult{}, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tc.spent, purchaseSpent(tc.result, tc.err)) + }) + } +} From 111664314dc50c1e0e0c2df9cb3b2b0a26224e33 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 4 Nov 2026 08:47:34 +0100 Subject: [PATCH 3/4] docs: document budget release on pre-purchase refusal and the Azure Advisor change Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 9 +++++++++ README.md | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 63cee81..833f396 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -65,6 +65,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### Changed +- Bumped the `cloud-commitments-go` pins to 39dc6e7. Azure Advisor + recommendations are now priced through the Retail Prices Pricer and skipped + when they cannot be priced; rows already covered by Consumption are dropped, + so Azure search results can shrink. - Bumped the `cloud-commitments-go` pins to 8785de4 for the EC2 approved-upfront guard (go#611). `provider.ProviderConfig` is now a deprecated alias of `provider.Config`; the call sites use `provider.Config`. Behavior pulled in: @@ -125,6 +129,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### Fixed +- A real purchase the provider refuses before any purchase call (for example the + EC2 approved-upfront price guard, which wraps + `common.ErrRefusedBeforePurchase`) no longer stays counted in + `CUDLY_MCP_BUDGET_USD`. Errors that may follow a real purchase, or that also + wrap `ErrOutcomeUnknown`, stay counted. - The search's dedupe failure suggests the missing IAM permission only when listing existing commitments was refused (AWS `AccessDenied`, `UnauthorizedOperation`; Azure or GCP 401/403). Other listing errors, such as diff --git a/README.md b/README.md index 82a9a71..827cdfc 100644 --- a/README.md +++ b/README.md @@ -125,7 +125,7 @@ Every other provider's purchase tool (`cudly_aws_savingsplans_purchase`, `cudly_ - A real `partial-upfront` or `all-upfront` EC2 RI purchase requires `approved_upfront` and `approved_currency` copied from its preview. They are passed to the provider as the approved upfront, and the provider refuses to buy an offering whose upfront (`FixedPrice` x `count`) exceeds it by more than 5%, so a price change between preview and purchase cannot buy at the higher price. The figure is caller-supplied: it bounds price drift, not a deliberately inflated figure, which the USD cap and budget below still bound. `approved_upfront` must be a positive number and `approved_currency` must be `USD`; both are rejected for `no-upfront`. - `dry_run=true` always previews, including when `confirm=true`. A real purchase requires **both** `dry_run=false` **and** `confirm=true`; `dry_run=false` with `confirm=false` is refused with a structured error, not silently downgraded to a preview. - A real purchase **also** requires the operator to have set `CUDLY_MCP_ENABLE_REAL_PURCHASES=1` (or `true`, case-insensitive) in the environment `cudly-mcp` was launched with. This is layered underneath `confirm`: `confirm` only proves the model asked to spend money, it does not prove the operator running this server wants it able to. The gate is unset (disabled) by default -- unset, empty, `0`, `false`, or any other value all disable real purchases -- so a fresh install cannot spend money until the operator explicitly opts in. When disabled, a `dry_run=false, confirm=true` call is refused before any provider or credential is touched, naming the flag to set. Dry runs are unaffected by this flag; they never spend regardless of its value. -- A real purchase also requires **operator-set spend caps**, read from the server's environment (never from tool arguments, which the model controls). Unset, empty or invalid caps refuse the purchase, naming the variable; there is no default and no "unlimited". `CUDLY_MCP_MAX_COMMITMENT_USD` (positive number, USD) is required for every real purchase and bounds the **total USD one call commits over the full term**: it is read from the provider's own offering lookup just before buying (Savings Plans: hourly commitment x 8760 x years, computed locally), so executing needs credentials and network access for that lookup. Only USD is supported; a purchase whose price is missing, zero, non-USD or errors is refused rather than assumed cheap. Real GCP CUD purchases are refused until the `cloud-commitments-go` pin includes the CUD pricing fix (go#321). The Azure figure is the Retail Prices list price, not a negotiated rate. The lookup and the purchase are separate calls, so the price can change between them. `CUDLY_MCP_MAX_COUNT` (positive integer) bounds `count` for every RI purchase and `vcpu_count` for GCP CUDs. `CUDLY_MCP_MAX_HOURLY_COMMITMENT` (positive number, USD/hour) bounds Savings Plan `hourly_commitment`. `CUDLY_MCP_MAX_MEMORY_GB` (positive number) bounds `memory_gb` for GCP CUDs. Previews need no caps. `CUDLY_MCP_BUDGET_USD` (positive number, USD) is also required and is a **cumulative** budget: the USD totals of all real purchases in this server process may not exceed it, so a fresh `idempotency_nonce`, region or profile cannot repeat a purchase past it. **Limits you must understand:** the other caps apply **per call**. By default the budget is held in memory, so it resets when the server restarts and is not shared across processes or MCP clients; set `CUDLY_MCP_BUDGET_STATE` to persist it (see [Persisted budget](#persisted-budget)). Either way it cannot see purchases made in the cloud console, CLI or web. A purchase whose outcome is unknown, or that fails after the provider was called, counts as spent; only a clean no-op is released. An existing commitment the library adopts on an idempotent retry is released, but the retry still needs headroom for its own total at the time of the call. Choose the USD cap as the most one call may ever spend. +- A real purchase also requires **operator-set spend caps**, read from the server's environment (never from tool arguments, which the model controls). Unset, empty or invalid caps refuse the purchase, naming the variable; there is no default and no "unlimited". `CUDLY_MCP_MAX_COMMITMENT_USD` (positive number, USD) is required for every real purchase and bounds the **total USD one call commits over the full term**: it is read from the provider's own offering lookup just before buying (Savings Plans: hourly commitment x 8760 x years, computed locally), so executing needs credentials and network access for that lookup. Only USD is supported; a purchase whose price is missing, zero, non-USD or errors is refused rather than assumed cheap. Real GCP CUD purchases are refused until the `cloud-commitments-go` pin includes the CUD pricing fix (go#321). The Azure figure is the Retail Prices list price, not a negotiated rate. The lookup and the purchase are separate calls, so the price can change between them. `CUDLY_MCP_MAX_COUNT` (positive integer) bounds `count` for every RI purchase and `vcpu_count` for GCP CUDs. `CUDLY_MCP_MAX_HOURLY_COMMITMENT` (positive number, USD/hour) bounds Savings Plan `hourly_commitment`. `CUDLY_MCP_MAX_MEMORY_GB` (positive number) bounds `memory_gb` for GCP CUDs. Previews need no caps. `CUDLY_MCP_BUDGET_USD` (positive number, USD) is also required and is a **cumulative** budget: the USD totals of all real purchases in this server process may not exceed it, so a fresh `idempotency_nonce`, region or profile cannot repeat a purchase past it. **Limits you must understand:** the other caps apply **per call**. By default the budget is held in memory, so it resets when the server restarts and is not shared across processes or MCP clients; set `CUDLY_MCP_BUDGET_STATE` to persist it (see [Persisted budget](#persisted-budget)). Either way it cannot see purchases made in the cloud console, CLI or web. A purchase whose outcome is unknown, or that fails after the provider was called, counts as spent. Two cases are released: a clean no-op, and a purchase the provider refused before any purchase call (for example the approved-upfront price guard), which it marks with `common.ErrRefusedBeforePurchase`. An existing commitment the library adopts on an idempotent retry is released, but the retry still needs headroom for its own total at the time of the call. Choose the USD cap as the most one call may ever spend. - A real purchase requires the **target account to be named**: `aws_profile` (AWS) or `azure_subscription_id` (Azure), either as the tool argument or via the matching environment variable the provider itself reads (`AWS_PROFILE`, `AZURE_SUBSCRIPTION_ID`). **GCP has no environment fallback, so `gcp_project_id` is required for a real CUD purchase.** That is deliberate: nothing in `providers/gcp` reads `GOOGLE_CLOUD_PROJECT` or `CLOUDSDK_CORE_PROJECT`, and with no project configured the provider falls back to the *first active project* in your `ListProjects` response -- an artifact of IAM visibility and API ordering rather than a project anyone chose, which is not a defensible default for spending money. Pointing the scope at an environment variable the provider ignores would be worse still: the idempotency token would name one project while the commitment landed in another. If neither is set, the real purchase is refused before any credential is touched, naming the argument to pass. **Dry runs do not require it** -- you can price a purchase without naming an account. This is not bookkeeping: the account is folded into the idempotency token, so leaving it to ambient credentials on one call and naming it explicitly on the next derives two *different* tokens for the *same* account. Every provider's dedupe is token-keyed, so the second call's lookup would miss and buy a second commitment. Refusing an undeterminable account makes that aliasing unreachable. Naming an account explicitly and inheriting the same value from the environment always agree, so they still dedupe normally. - Every money-affecting parameter (region, resource type, count, term, payment option, and any provider-specific dimension such as RDS's `az_config`) is validated against an explicit enum or non-empty check before anything is built or sent. There is no silent default for a value that materially changes what gets purchased. - Every real purchase is tagged with a source identifying it came from this MCP server (never a user-suppliable string) and a deterministic idempotency token derived from the request's own parameters. By default, retrying an identical tool call -- however long after the original, and regardless of any clock boundary -- always derives the same token, so the provider dedupes the retry instead of buying twice; this is a fail-safe default, since the worst case of a false dedupe is a skipped intentional repeat, never a double purchase. To deliberately make a second, otherwise-identical purchase (e.g. "buy 3 RIs now" and "buy 3 more next week"), pass a fresh `idempotency_nonce` value on the second call; passing the same nonce on a retry of that same call still dedupes correctly. From 6a7d197112663aedd16bb55a75ed49249a1b989f Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 4 Nov 2026 08:56:25 +0100 Subject: [PATCH 4/4] fix(budget): keep an adopted existing commitment counted on a refusal Also note the "refused before purchase: " prefix in the CHANGELOG. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 3 ++- tools/budget.go | 2 +- tools/budget_test.go | 1 + 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 833f396..89a3dc2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,7 +68,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). - Bumped the `cloud-commitments-go` pins to 39dc6e7. Azure Advisor recommendations are now priced through the Retail Prices Pricer and skipped when they cannot be priced; rows already covered by Consumption are dropped, - so Azure search results can shrink. + so Azure search results can shrink. The approved-upfront guard errors the + AWS RI purchase tools return now start with "refused before purchase: ". - Bumped the `cloud-commitments-go` pins to 8785de4 for the EC2 approved-upfront guard (go#611). `provider.ProviderConfig` is now a deprecated alias of `provider.Config`; the call sites use `provider.Config`. Behavior pulled in: diff --git a/tools/budget.go b/tools/budget.go index e67da1c..1aadb00 100644 --- a/tools/budget.go +++ b/tools/budget.go @@ -104,7 +104,7 @@ func purchaseSpent(result common.PurchaseResult, err error) bool { } func refusedBeforePurchase(result common.PurchaseResult, err error) bool { - if result.Success || result.CommitmentID != "" { + if result.Success || result.CommitmentID != "" || result.ExistingCommitment { return false } for _, e := range []error{err, result.Error} { diff --git a/tools/budget_test.go b/tools/budget_test.go index c0f8ca4..6096ade 100644 --- a/tools/budget_test.go +++ b/tools/budget_test.go @@ -178,6 +178,7 @@ func TestPurchaseSpent(t *testing.T) { {assert.AnError, "sentinel in result.Error, plain err", common.PurchaseResult{Error: refused}, true}, {unknown, "sentinel with outcome unknown", common.PurchaseResult{}, true}, {refused, "sentinel with success", common.PurchaseResult{Success: true}, true}, + {refused, "sentinel with existing commitment", common.PurchaseResult{ExistingCommitment: true}, true}, {refused, "sentinel with commitment id", common.PurchaseResult{CommitmentID: "ri-1"}, true}, {assert.AnError, "plain error", common.PurchaseResult{}, true}, {nil, "nil error with success", common.PurchaseResult{Success: true}, true},