diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ef5e87..5bf6413 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 + 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 `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..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": 1, + "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": 1, + "min": 0, "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..832e756 --- /dev/null +++ b/tools/budget_settle_test.go @@ -0,0 +1,84 @@ +package tools + +import ( + "context" + "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 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 := 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) +} 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])