From 55a4e205c43410436a5fff68a057a7cf924e1869 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 21 Oct 2026 00:22:59 +0200 Subject: [PATCH 1/2] fix(mcp): align budget manifest minimums with the server and pin settle on panic/cancel The manifest declared min 1 for budget_usd and max_commitment_usd while the server accepts any positive amount; the manifest now allows 0.01. Add tests that a panic or a canceled context inside PurchaseCommitment keeps the reservation counted, and that a pre-canceled context reserves nothing. The integer-cents ledger from mcp#82 item 2 is not part of this change: it alters rounding of stored USD amounts and the state file format. Part of #82 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 4 ++ mcpb/manifest.json | 4 +- tools/budget_settle_test.go | 111 ++++++++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 2 deletions(-) create mode 100644 tools/budget_settle_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ef5e87..3ea1077 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -110,6 +110,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### Fixed +- The Claude Desktop settings for `budget_usd` and `max_commitment_usd` now + accept down to 0.01 USD instead of 1, matching the server, which accepts any + positive amount (mcp#82). Tests also pin that a panic or a cancelled context + inside the provider purchase call keeps the budget reservation counted. - `cudly_aws_ec2_ri_purchase` with `offering_class=standard` is now capped by `CUDLY_MCP_MAX_COMMITMENT_USD` and counted against `CUDLY_MCP_BUDGET_USD` at the standard offering's price (`GetOfferingDetailsForClass`, go#574), not the diff --git a/mcpb/manifest.json b/mcpb/manifest.json index 60489c4..b753eac 100644 --- a/mcpb/manifest.json +++ b/mcpb/manifest.json @@ -102,14 +102,14 @@ "type": "number", "title": "Maximum commitment per purchase call (USD)", "description": "Per-call ceiling on the total USD one purchase commits over its full term, priced by the provider just before buying. Required before any real purchase: leave it empty and purchases are refused. USD only; purchases that cannot be priced (including real GCP CUD purchases for now) are refused. Applies per call, not cumulatively.", - "min": 1, + "min": 0.01, "required": false }, "budget_usd": { "type": "number", "title": "Cumulative purchase budget per server process (USD)", "description": "Total USD that real purchases may commit over their full terms in one server process. Required before any real purchase: leave it empty and purchases are refused. Held in memory: it resets when the server restarts, is not shared across processes or MCP clients, and cannot see purchases made in the cloud console, CLI or elsewhere. A purchase with an unknown outcome counts as spent.", - "min": 1, + "min": 0.01, "required": false }, "archera_api_key": { diff --git a/tools/budget_settle_test.go b/tools/budget_settle_test.go new file mode 100644 index 0000000..2a42a59 --- /dev/null +++ b/tools/budget_settle_test.go @@ -0,0 +1,111 @@ +package tools + +import ( + "context" + "encoding/json" + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" + "github.com/LeanerCloud/cloud-commitments-go/pkg/provider" +) + +// hookClient lets a test run code inside PurchaseCommitment. +type hookClient struct { + *fakeServiceClient + onPurchase func(context.Context) (common.PurchaseResult, error) +} + +func (c hookClient) PurchaseCommitment(ctx context.Context, _ common.Recommendation, _ common.PurchaseOptions) (common.PurchaseResult, error) { + return c.onPurchase(ctx) +} + +func runWithContext(ctx context.Context, fake provider.ServiceClient) error { + rec := testRecommendation() + rec.Count = 1 + _, err := ExecutePurchase(ctx, PurchaseRequest{ + Region: rec.Region, Recommendation: rec, Confirm: true, CredentialScope: "scope", Nonce: "n", + ResolveClient: func(context.Context) (provider.ServiceClient, error) { return fake, nil }, + }) + return err +} + +// A panic inside the provider call must leave the reservation held: the buy +// may have gone through, so the budget fails closed. +func TestBudgetPanicInPurchaseStaysReserved(t *testing.T) { + resetBudget(t, "100") + fake := hookClient{pricedFake(60, common.PurchaseResult{}, nil), func(context.Context) (common.PurchaseResult, error) { + panic("provider blew up") + }} + require.Panics(t, func() { _ = runWithContext(context.Background(), fake) }) + + processBudget.mu.Lock() + reserved := processBudget.reserved + processBudget.mu.Unlock() + assert.InDelta(t, 60, reserved, 1e-9) + + other := pricedFake(60, common.PurchaseResult{Success: true, CommitmentID: "id"}, nil) + require.ErrorContains(t, runWithNonce(other, "b"), EnvBudgetUSD) + assert.Zero(t, other.purchaseCalls) +} + +// Canceling the context while the provider call is in flight is an error +// outcome: the buy may have happened, so it stays counted. +func TestBudgetCancelDuringPurchaseStaysCounted(t *testing.T) { + resetBudget(t, "100") + ctx, cancel := context.WithCancel(context.Background()) + fake := hookClient{pricedFake(60, common.PurchaseResult{}, nil), func(ctx context.Context) (common.PurchaseResult, error) { + cancel() + return common.PurchaseResult{}, ctx.Err() + }} + require.ErrorIs(t, runWithContext(ctx, fake), context.Canceled) + assert.InDelta(t, 60, processBudget.committedUSD(), 1e-9) + + other := pricedFake(60, common.PurchaseResult{Success: true, CommitmentID: "id"}, nil) + require.ErrorContains(t, runWithNonce(other, "b"), EnvBudgetUSD) +} + +// A context canceled before the purchase starts is a refusal that reserves nothing. +func TestBudgetCancelBeforePurchaseReservesNothing(t *testing.T) { + resetBudget(t, "100") + ctx, cancel := context.WithCancel(context.Background()) + cancel() + fake := pricedFake(60, common.PurchaseResult{Success: true, CommitmentID: "id"}, nil) + fake.offeringErr = ctx.Err() + require.Error(t, runWithContext(ctx, fake)) + assert.Zero(t, fake.purchaseCalls) + processBudget.mu.Lock() + defer processBudget.mu.Unlock() + assert.Zero(t, processBudget.reserved) + assert.Zero(t, processBudget.committed) +} + +// The manifest bounds for the two USD caps must not be stricter than the +// server, which accepts any positive amount: a sub-dollar cap is valid. +func TestManifestUSDCapMinimumMatchesServer(t *testing.T) { + data, err := os.ReadFile("../mcpb/manifest.json") + require.NoError(t, err) + var m struct { + UserConfig map[string]struct { + Min *float64 `json:"min"` + } `json:"user_config"` + } + require.NoError(t, json.Unmarshal(data, &m)) + for key, env := range map[string]string{"budget_usd": EnvBudgetUSD, "max_commitment_usd": EnvMaxCommitmentUSD} { + entry, ok := m.UserConfig[key] + require.True(t, ok, key) + require.NotNil(t, entry.Min, key) + t.Setenv(env, "0.01") + var got float64 + if env == EnvBudgetUSD { + got, err = readBudget() + } else { + got, err = readUSDCap() + } + require.NoError(t, err, key) + assert.LessOrEqual(t, *entry.Min, got, "%s: manifest min must not exceed the smallest value the server accepts", key) + } +} From 4ca980a1f4bcf5cf6aca01c8279571f15c1f7f14 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 21 Oct 2026 01:40:04 +0200 Subject: [PATCH 2/2] fix(mcp): use the inclusive min 0 for the USD caps and make the cancel test cancel The repo's manifest bounds rule is an inclusive min of 0 for float caps, so budget_usd and max_commitment_usd use 0 (the server still rejects 0). Both caps join TestManifestMinMatchesServerCapBounds and the weaker duplicate test is removed. The pre-provider cancel test now cancels the context after the reservation, so a mutant that releases on cancel fails it. Part of #82 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- mcpb/manifest.json | 4 ++-- tools/budget_settle_test.go | 45 +++++++---------------------------- tools/manifest_bounds_test.go | 3 +-- 4 files changed, 13 insertions(+), 41 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ea1077..5bf6413 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -111,7 +111,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### Fixed - The Claude Desktop settings for `budget_usd` and `max_commitment_usd` now - accept down to 0.01 USD instead of 1, matching the server, which accepts any + use the inclusive min 0 (like the other float caps) instead of 1, so sub-dollar amounts are accepted, matching the server, which accepts any positive amount (mcp#82). Tests also pin that a panic or a cancelled context inside the provider purchase call keeps the budget reservation counted. - `cudly_aws_ec2_ri_purchase` with `offering_class=standard` is now capped by diff --git a/mcpb/manifest.json b/mcpb/manifest.json index b753eac..a3e3c84 100644 --- a/mcpb/manifest.json +++ b/mcpb/manifest.json @@ -102,14 +102,14 @@ "type": "number", "title": "Maximum commitment per purchase call (USD)", "description": "Per-call ceiling on the total USD one purchase commits over its full term, priced by the provider just before buying. Required before any real purchase: leave it empty and purchases are refused. USD only; purchases that cannot be priced (including real GCP CUD purchases for now) are refused. Applies per call, not cumulatively.", - "min": 0.01, + "min": 0, "required": false }, "budget_usd": { "type": "number", "title": "Cumulative purchase budget per server process (USD)", "description": "Total USD that real purchases may commit over their full terms in one server process. Required before any real purchase: leave it empty and purchases are refused. Held in memory: it resets when the server restarts, is not shared across processes or MCP clients, and cannot see purchases made in the cloud console, CLI or elsewhere. A purchase with an unknown outcome counts as spent.", - "min": 0.01, + "min": 0, "required": false }, "archera_api_key": { diff --git a/tools/budget_settle_test.go b/tools/budget_settle_test.go index 2a42a59..832e756 100644 --- a/tools/budget_settle_test.go +++ b/tools/budget_settle_test.go @@ -2,8 +2,6 @@ package tools import ( "context" - "encoding/json" - "os" "testing" "github.com/stretchr/testify/assert" @@ -68,44 +66,19 @@ func TestBudgetCancelDuringPurchaseStaysCounted(t *testing.T) { require.ErrorContains(t, runWithNonce(other, "b"), EnvBudgetUSD) } -// A context canceled before the purchase starts is a refusal that reserves nothing. -func TestBudgetCancelBeforePurchaseReservesNothing(t *testing.T) { +// A context canceled before the provider call: pricing ignores it, the budget +// is reserved, and the provider call returns the cancellation. Whether it +// bought anything is unknown, so the reservation stays counted. +func TestBudgetCanceledBeforeProviderCallStaysCounted(t *testing.T) { resetBudget(t, "100") ctx, cancel := context.WithCancel(context.Background()) cancel() - fake := pricedFake(60, common.PurchaseResult{Success: true, CommitmentID: "id"}, nil) - fake.offeringErr = ctx.Err() - require.Error(t, runWithContext(ctx, fake)) - assert.Zero(t, fake.purchaseCalls) + fake := hookClient{pricedFake(60, common.PurchaseResult{}, nil), func(ctx context.Context) (common.PurchaseResult, error) { + return common.PurchaseResult{}, ctx.Err() + }} + require.ErrorIs(t, runWithContext(ctx, fake), context.Canceled) + assert.InDelta(t, 60, processBudget.committedUSD(), 1e-9) processBudget.mu.Lock() defer processBudget.mu.Unlock() assert.Zero(t, processBudget.reserved) - assert.Zero(t, processBudget.committed) -} - -// The manifest bounds for the two USD caps must not be stricter than the -// server, which accepts any positive amount: a sub-dollar cap is valid. -func TestManifestUSDCapMinimumMatchesServer(t *testing.T) { - data, err := os.ReadFile("../mcpb/manifest.json") - require.NoError(t, err) - var m struct { - UserConfig map[string]struct { - Min *float64 `json:"min"` - } `json:"user_config"` - } - require.NoError(t, json.Unmarshal(data, &m)) - for key, env := range map[string]string{"budget_usd": EnvBudgetUSD, "max_commitment_usd": EnvMaxCommitmentUSD} { - entry, ok := m.UserConfig[key] - require.True(t, ok, key) - require.NotNil(t, entry.Min, key) - t.Setenv(env, "0.01") - var got float64 - if env == EnvBudgetUSD { - got, err = readBudget() - } else { - got, err = readUSDCap() - } - require.NoError(t, err, key) - assert.LessOrEqual(t, *entry.Min, got, "%s: manifest min must not exceed the smallest value the server accepts", key) - } } diff --git a/tools/manifest_bounds_test.go b/tools/manifest_bounds_test.go index 8eb5e4c..5d79e29 100644 --- a/tools/manifest_bounds_test.go +++ b/tools/manifest_bounds_test.go @@ -42,11 +42,10 @@ func TestManifestMinMatchesServerCapBounds(t *testing.T) { return err == nil } - // The USD totals (capUSD, capBudget) are tracked by mcp#82. caps := []struct { c capSetting isInt bool - }{{capCount, true}, {capHourly, false}, {capMemory, false}} + }{{capCount, true}, {capHourly, false}, {capMemory, false}, {capUSD, false}, {capBudget, false}} for _, tc := range caps { t.Run(tc.c.env, func(t *testing.T) { sub := ref.FindStringSubmatch(m.Server.MCPConfig.Env[tc.c.env])