Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,15 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).

## [Unreleased]

### Changed

- `cudly_search_recommendations` now checks results against commitments bought
in the last 24 hours and returns a `dedupe` block: fully covered RDS,
OpenSearch, Redshift and ElastiCache recommendations are suppressed and
listed, possibly covered EC2, Azure, GCP and MemoryDB ones are flagged.
The search now needs permission to list existing commitments and fails
loudly, naming it, when it cannot.

### Breaking

- **Real purchases now require operator spend caps.** With
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ These are pre-existing behaviours in the underlying purchase clients, not someth
- **GCP Compute Engine CUDs commit resources, not instances.** `cudly_gcp_computeengine_cud_purchase` takes `vcpu_count` and `memory_gb` directly (a CUD is a vCPU+memory commitment), not an instance count -- there is no implicit vCPU-per-instance conversion.
- **AWS Savings Plans searches require term/payment/lookback; `cudly_search_recommendations` defaults them.** Unlike EC2/RDS reservation searches, `GetSavingsPlansPurchaseRecommendation` rejects the call unless all three are set. When `service` targets a Savings Plans search on AWS, the tool fills in unset fields with `term_years=1` (1yr), `payment_option=no-upfront`, `lookback_period=30d` -- an explicitly supplied value is never overridden. A Savings Plans search therefore resolves to exactly one (term, payment, lookback) triple and issues exactly one Cost Explorer call.
- **Reservation searches fan out over every term and payment option when you omit them.** `GetReservationPurchaseRecommendation` accepts exactly one `TermInYears` and one `PaymentOption` per request and returns recommendations only for that cell -- there is no "give me every variant" mode. So when `term_years` and/or `payment_option` are omitted on an EC2/RDS/ElastiCache/etc search, the tool expands the omitted dimension to its full menu and issues one call per combination (6 when both are omitted, 2 or 3 when one is), returning the concatenated results. Each returned recommendation carries the `term`/`payment_option` of the request that produced it, so its money figures are attributable to a specific offer. If any one combination fails, the whole search fails rather than returning a partial menu -- five of six offers is indistinguishable from "these are all your options". Note that `lookback_period` is NOT fanned out: omitting it leaves Cost Explorer's own server-side default (7 days) in place, since the lookback window is the usage evidence behind an offer rather than another offer to choose from. The fan-out multiplies the API bill along with the coverage: `cudly_search_recommendations` buys no commitment, but Cost Explorer bills per request, so an omitted-term/omitted-payment search costs six requests rather than one (more with pagination). The tool description states this too, so a model does not treat searching as free and re-run it in a loop.
- **`cudly_search_recommendations` checks results against recent commitments, with limits.** After fetching recommendations, the tool lists the commitments already bought (active, payment-pending or queued, started within the last 24 hours) for each service and region, using the credentials of the search. A recommendation fully covered by them is removed from `recommendations` and listed in `dedupe.suppressed` (service, region, resource type, account, term, payment option, count, reason). Suppression applies only to RDS, OpenSearch, Redshift and ElastiCache (when the commitment's engine is known) and only for recommendations in the credentials' own AWS account. For EC2, Azure, GCP and MemoryDB the matching key is too coarse to hide anything, so a possibly covered recommendation stays and appears in `dedupe.flagged` as `possibly_covered`; a partly covered one is returned unchanged (same count, cost and savings) with `covered_count`. Savings Plans are not checked. The six term/payment variants of one demand are all treated alike. Limits: purchases in other accounts, and anything older than 24 hours, are not seen; RDS size flexibility is not detected, so a covered recommendation can still be shown; the key ignores term and payment option. If listing commitments fails (for example for lack of `rds:DescribeReservedDBInstances`), the whole search fails and names the permission, rather than returning unchecked results. The key gaps are tracked in <https://github.com/LeanerCloud/cloud-commitments-go/issues/323>.
- **`region` on `cudly_search_recommendations` is a client-side post-filter for AWS reservation/Savings Plans searches.** `GetReservationPurchaseRecommendation` and `GetSavingsPlansPurchaseRecommendation` are account-level Cost Explorer calls with no region parameter -- AWS returns recommendations from every region the account has usage in. The tool filters the returned recommendations down to `region`/`include_regions` (minus `exclude_regions`) after the call; when no region constraint is supplied, all regions are returned as before.

## Deployment model
Expand Down
135 changes: 135 additions & 0 deletions cmd/cudly-mcp/search_dedupe_protocol_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
package main

import (
"context"
"encoding/json"
"errors"
"testing"
"time"

"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"
awsprovider "github.com/LeanerCloud/cloud-commitments-go/providers/aws"

cudlymcp "github.com/LeanerCloud/cloud-commitments-mcp"

gosdk "github.com/modelcontextprotocol/go-sdk/mcp"
)

const searchCallerAccount = "111111111111"

func searchProtocolRec() common.Recommendation {
return common.Recommendation{
Provider: common.ProviderAWS, Service: common.ServiceRDS, Account: searchCallerAccount, Region: "us-east-1",
ResourceType: "db.r6g.large", Count: 2, Term: "1yr", PaymentOption: "all-upfront",
Details: &common.DatabaseDetails{Engine: "MySQL", AZConfig: "multi-az"},
}
}

// searchRecsClient serves a fixed recommendation list.
type searchRecsClient struct{ provider.RecommendationsClient }

func (searchRecsClient) GetRecommendations(_ context.Context, _ *common.RecommendationParams) ([]common.Recommendation, error) {
return []common.Recommendation{searchProtocolRec()}, nil
}

// recentRDSClient lists one recent reservation that covers the recommendation.
type recentRDSClient struct{ provider.ServiceClient }

func (recentRDSClient) GetExistingCommitments(_ context.Context) ([]common.Commitment, error) {
return []common.Commitment{{
Provider: common.ProviderAWS, Service: common.ServiceRelationalDB, Region: "us-east-1", ResourceType: "db.r6g.large",
Engine: "mysql", Deployment: "multi-az", Count: 2, State: common.CommitmentStateActive,
StartDate: time.Now().Add(-time.Hour),
}}, nil
}

type fakeSearchAWSProvider struct{}

func (fakeSearchAWSProvider) Name() string { return "aws" }
func (fakeSearchAWSProvider) DisplayName() string { return "Amazon Web Services" }
func (fakeSearchAWSProvider) IsConfigured() bool { return true }
func (fakeSearchAWSProvider) GetCredentials() (provider.Credentials, error) {
return nil, errors.New("not implemented")
}
func (fakeSearchAWSProvider) ValidateCredentials(_ context.Context) error {
return errors.New("not implemented")
}
func (fakeSearchAWSProvider) GetAccounts(_ context.Context) ([]common.Account, error) {
return []common.Account{{ID: searchCallerAccount, IsDefault: true}}, nil
}
func (fakeSearchAWSProvider) GetRegions(_ context.Context) ([]common.Region, error) {
return nil, errors.New("not implemented")
}
func (fakeSearchAWSProvider) GetDefaultRegion() string { return "us-east-1" }
func (fakeSearchAWSProvider) GetSupportedServices() []common.ServiceType {
return []common.ServiceType{common.ServiceRDS}
}
func (fakeSearchAWSProvider) GetServiceClient(_ context.Context, service common.ServiceType, _ string) (provider.ServiceClient, error) {
if service != common.ServiceRDS {
return nil, errors.New("only rds is implemented")
}
return recentRDSClient{}, nil
}
func (fakeSearchAWSProvider) GetRecommendationsClient(_ context.Context) (provider.RecommendationsClient, error) {
return searchRecsClient{}, nil
}

// TestSearchSuppressesCoveredRecommendationOverTheProtocol drives the search
// through the real server and an in-memory MCP client, with a fake aws
// provider: a recommendation covered by a recent reservation must leave the
// result and appear in dedupe.suppressed, and the structured output must pass
// the SDK's output-schema validation.
func TestSearchSuppressesCoveredRecommendationOverTheProtocol(t *testing.T) {
provider.GetRegistry().Unregister("aws")
require.NoError(t, provider.RegisterProvider("aws", func(_ *provider.ProviderConfig) (provider.Provider, error) {
return fakeSearchAWSProvider{}, nil
}))
t.Cleanup(func() {
provider.GetRegistry().Unregister("aws")
if err := provider.RegisterProvider("aws", func(cfg *provider.ProviderConfig) (provider.Provider, error) {
return awsprovider.NewAWSProvider(cfg)
}); err != nil {
t.Errorf("restore real aws provider factory: %v", err)
}
})

server, err := cudlymcp.NewServer("test-search-dedupe")
require.NoError(t, err)
clientTransport, serverTransport := gosdk.NewInMemoryTransports()
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
go func() { _ = server.Run(ctx, serverTransport) }()

client := gosdk.NewClient(&gosdk.Implementation{Name: "test-client"}, nil)
session, err := client.Connect(ctx, clientTransport, nil)
require.NoError(t, err)
defer session.Close()

result, err := session.CallTool(ctx, &gosdk.CallToolParams{
Name: "cudly_search_recommendations",
Arguments: map[string]any{
"provider": "aws", "service": "rds", "region": "us-east-1", "term_years": 1, "payment_option": "all-upfront",
},
})
require.NoError(t, err)
require.False(t, result.IsError, "%+v", result.Content)

encoded, err := json.Marshal(result.StructuredContent)
require.NoError(t, err)
var out struct {
Dedupe struct {
Suppressed []map[string]any `json:"suppressed"`
} `json:"dedupe"`
Recommendations []any `json:"recommendations"`
Count int `json:"count"`
}
require.NoError(t, json.Unmarshal(encoded, &out), "%s", encoded)
assert.Equal(t, 0, out.Count, "%s", encoded)
assert.Empty(t, out.Recommendations, "%s", encoded)
require.Len(t, out.Dedupe.Suppressed, 1, "%s", encoded)
assert.Equal(t, searchCallerAccount, out.Dedupe.Suppressed[0]["account"])
}
6 changes: 3 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ require (
github.com/aws/aws-sdk-go-v2/config v1.29.12 // indirect
github.com/aws/aws-sdk-go-v2/service/costexplorer v1.61.0 // indirect
github.com/aws/aws-sdk-go-v2/service/ec2 v1.251.2
github.com/aws/aws-sdk-go-v2/service/elasticache v1.50.3 // indirect
github.com/aws/aws-sdk-go-v2/service/elasticache v1.50.3
github.com/aws/aws-sdk-go-v2/service/memorydb v1.31.4 // indirect
github.com/aws/aws-sdk-go-v2/service/opensearch v1.52.3 // indirect
github.com/aws/aws-sdk-go-v2/service/opensearch v1.52.3
github.com/aws/aws-sdk-go-v2/service/rds v1.97.3
github.com/aws/aws-sdk-go-v2/service/redshift v1.58.3 // indirect
github.com/aws/aws-sdk-go-v2/service/redshift v1.58.3
github.com/stretchr/testify v1.11.1
)

Expand Down
5 changes: 2 additions & 3 deletions tools/gcp_computeengine_cud_safeguards_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,8 @@ func TestGCPComputeEnginePurchaseReportsExplicitZeroCost(t *testing.T) {
// FilterRecommendationsForRecentCommitments, so this recommendation passed
// through; at a32fd1a the family pool match filters it.
//
// Deliberately NOT protocol-level: this consumer has no dedupe path of its
// own (nothing here imports pkg/recfilter outside this test); the wiring gap
// is tracked as issue #42.
// This pins the library boundary; the search tool's own use of the checker
// is covered in search_dedupe_test.go.
func TestGCPComputeEngineDedupeDefersFamilyWithRecentCUD(t *testing.T) {
ctx := context.Background()

Expand Down
Loading
Loading