diff --git a/CHANGELOG.md b/CHANGELOG.md index 05fb1c8..7aa2dc4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### Changed +- The purchase audit log records a new status, `unknown`, for a purchase that + failed with `common.ErrOutcomeUnknown` (a lost EC2 or Redshift response: the + commitment may exist). The stderr line reads `OUTCOME UNKNOWN`. Consumers + of the JSONL that switch on `status` must handle the new value. - `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 diff --git a/README.md b/README.md index 5758d27..9d722d1 100644 --- a/README.md +++ b/README.md @@ -148,7 +148,7 @@ Auditing is **on by default**. When auditing is enabled and a write succeeds, ea - **`CUDLY_MCP_AUDIT_LOG`** overrides the path. Setting it to an **empty or whitespace-only string disables the log entirely** -- that is the explicit opt-out; leaving the variable unset is not the same thing and still uses the default path. - A preview (`dry_run=true`) is recorded with `status: "skipped"`, `dry_run: true` -- it spent nothing, but it was still a decision worth reconstructing later. - When a credential scope is supplied, `credential_scope` records that routing identifier: an AWS profile, Azure subscription, or GCP project. It identifies how CUDly selected the target; it is not a verified provider account ID. Previews may omit it because they do not require a target. -- A real purchase is recorded `"success"` only when the provider reports `Success: true` with no embedded error. A failed provider call, `Success: false`, or a result containing an error is recorded as `"error"`. +- A real purchase is recorded `"success"` only when the provider reports `Success: true` with no embedded error. A failed provider call, `Success: false`, or a result containing an error is recorded as `"error"`, except a failure that wraps the library's outcome-unknown error (an EC2 or Redshift purchase whose response was lost), which is recorded as `"unknown"`: the request may have reached the cloud and the commitment may exist, so reconcile with `DescribeReservedInstances` / `DescribeReservedNodes` before retrying. The stderr line for it reads `mcp purchase OUTCOME UNKNOWN` instead of `FAILED`. - Every purchase in one server process shares a single `run_id`, so a log spanning many purchases can be grouped by session. - **A write failure never changes the purchase result.** It logs a warning to stderr naming the path and error; the tool response returned to the caller is unaffected. - The path is probed for read, append, and directory durability at server startup. MCP creates missing parent directories as owner-only `0700` subject to umask, then opens and syncs every directory edge from the filesystem root. Each ancestor therefore needs search and read permission, every creation parent also needs write permission, and the filesystem must support directory `fsync`. A path that fails these checks stops server construction instead of silently dropping every record for the session. diff --git a/go.mod b/go.mod index 88bbd70..87908f4 100644 --- a/go.mod +++ b/go.mod @@ -77,10 +77,10 @@ require ( ) require ( - github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009134326-8a3d92b9ddae - github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261009134326-8a3d92b9ddae - github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261009134326-8a3d92b9ddae - github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261009134326-8a3d92b9ddae + github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261010025946-16b57954c6bc + github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261010025946-16b57954c6bc + github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261010025946-16b57954c6bc + github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261010025946-16b57954c6bc github.com/google/jsonschema-go v0.4.3 github.com/google/uuid v1.6.0 github.com/modelcontextprotocol/go-sdk v1.6.1 diff --git a/go.sum b/go.sum index ce5fdc6..985bebb 100644 --- a/go.sum +++ b/go.sum @@ -76,14 +76,14 @@ github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/cloudmock v0 github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/cloudmock v0.54.0/go.mod h1:vB2GH9GAYYJTO3mEn8oYwzEdhlayZIdQz6zdzgUIRvA= github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.54.0 h1:s0WlVbf9qpvkh1c/uDAPElam0WrL7fHRIidgZJ7UqZI= github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.54.0/go.mod h1:Mf6O40IAyB9zR/1J8nGDDPirZQQPbYJni8Yisy7NTMc= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009134326-8a3d92b9ddae h1:vDHLLIg/QAXOq/kAYbmExL914ZFV92zYrhajZM5Xx8A= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009134326-8a3d92b9ddae/go.mod h1:8RTmzPazA8IeVZfn7wNaU7ILx0VK5eHZQozO21Mvtq8= -github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261009134326-8a3d92b9ddae h1:90bEVU16HS/6N/76j3uW4dXuDSOrA1xEvwk5Q2chwFs= -github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261009134326-8a3d92b9ddae/go.mod h1:yJoe/zCikjjlGsy47BZTTuKUcLkLVDwqgLEYklWjlm4= -github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261009134326-8a3d92b9ddae h1:G5OuYJOYPT35ql59H4x/SduQKMtNXSW5vUBHUrz9JL4= -github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261009134326-8a3d92b9ddae/go.mod h1:OVthbPmsDATeLavlCT3w7UUpF/WJnvDf33pe3y1WXo8= -github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261009134326-8a3d92b9ddae h1:SW5+gQ4I3bRhluh0amQ2kBxSF9MiaFisVYwr6fc7Qkg= -github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261009134326-8a3d92b9ddae/go.mod h1:D2ZwYqlDm/304JWfOw8YX/e+/YTc51Q9kiIaHJN7nxY= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261010025946-16b57954c6bc h1:3V+lGgioy/qs02UceD458NP8j6AC9jkXUxjvG52eJgw= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261010025946-16b57954c6bc/go.mod h1:8RTmzPazA8IeVZfn7wNaU7ILx0VK5eHZQozO21Mvtq8= +github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261010025946-16b57954c6bc h1:IitorEN3jauxyrdwloOCyck9T0FycvXyyb5g/IN93VM= +github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261010025946-16b57954c6bc/go.mod h1:C0ildZeRds0aRvDZp49MLY0kFWh/4FiM4lPWWWd3HFA= +github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261010025946-16b57954c6bc h1:qitfqEIL5oiMSRMWOzvYqumNBnx+fqsXBSFiB7nmC6c= +github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261010025946-16b57954c6bc/go.mod h1:nM3Zqd5N2mMIsNoTtnKF6RFyM1E3HVjAGIsXVx9b3bY= +github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261010025946-16b57954c6bc h1:CrO7O548kjLkKAacbPcTnXYGn2gc1HXmZ+zwBjpFCYo= +github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261010025946-16b57954c6bc/go.mod h1:uJ7UpLaUqu1fpfCEECuqjLXV+As2tibtPCapPF3AZys= github.com/aws/aws-sdk-go-v2 v1.41.5 h1:dj5kopbwUsVUVFgO4Fi5BIT3t4WyqIDjGKCangnV/yY= github.com/aws/aws-sdk-go-v2 v1.41.5/go.mod h1:mwsPRE8ceUUpiTgF7QmQIJ7lgsKUPQOUl3o72QBrE1o= github.com/aws/aws-sdk-go-v2/config v1.29.12 h1:Y/2a+jLPrPbHpFkpAAYkVEtJmxORlXoo5k2g1fa2sUo= diff --git a/tools/audit.go b/tools/audit.go index d809018..0ad76c0 100644 --- a/tools/audit.go +++ b/tools/audit.go @@ -1,6 +1,7 @@ package tools import ( + "errors" "fmt" "log" "os" @@ -33,16 +34,19 @@ const EnvAuditLog = "CUDLY_MCP_AUDIT_LOG" // lifetime, matching how a single CLI invocation shares one run. var auditRunID = uuid.NewString() -// auditStatusSuccess, auditStatusError, and auditStatusSkipped are the three -// statuses this server ever writes. A dry run is skipped; a provider result -// is successful only when Success is true and Error is nil; every other -// provider outcome is an error. The fourth status common.NewAuditRecord +// auditStatusSuccess, auditStatusError, auditStatusUnknown and +// auditStatusSkipped are the statuses this server ever writes. A dry run is +// skipped; a provider result is successful only when Success is true and +// Error is nil; a failure wrapping common.ErrOutcomeUnknown is unknown (the +// request may have reached the cloud and the commitment may exist); every +// other provider outcome is an error. The fourth status common.NewAuditRecord // documents, "skipped_covered", belongs to the CLI's recent-duplicate guard, // which this server does not run yet. const ( auditStatusSuccess = "success" auditStatusError = "error" auditStatusSkipped = "skipped" + auditStatusUnknown = "unknown" ) // auditStatusFor maps a provider PurchaseResult to the audit status it @@ -52,6 +56,15 @@ func auditStatusFor(result common.PurchaseResult) string { if purchaseSucceeded(result) { return auditStatusSuccess } + return auditStatusForError(result.Error) +} + +// auditStatusForError maps a failed purchase's error to an audit status. The +// sentinel is matched with errors.Is, never by message text. +func auditStatusForError(err error) string { + if errors.Is(err, common.ErrOutcomeUnknown) { + return auditStatusUnknown + } return auditStatusError } diff --git a/tools/purchase.go b/tools/purchase.go index 567bee5..5df6fee 100644 --- a/tools/purchase.go +++ b/tools/purchase.go @@ -553,8 +553,12 @@ func logPurchaseOutcome(rec common.Recommendation, token, commitmentID string, s if err == nil { err = errors.New(providerFailureWithoutDetail) } - log.Printf("mcp purchase FAILED: provider=%s resource=%s token=%s: %v", - rec.Provider, rec.ResourceType, common.MaskToken(token), err) + label := "FAILED" + if errors.Is(err, common.ErrOutcomeUnknown) { + label = "OUTCOME UNKNOWN" + } + log.Printf("mcp purchase %s: provider=%s resource=%s token=%s: %v", + label, rec.Provider, rec.ResourceType, common.MaskToken(token), err) return } log.Printf("mcp purchase OK: provider=%s resource=%s count=%d commitment_id=%s token=%s", @@ -647,7 +651,7 @@ func ExecutePurchase(ctx context.Context, req PurchaseRequest) (*PurchaseRespons result, err := client.PurchaseCommitment(ctx, rec, opts) if err != nil { logPurchaseOutcome(rec, token, "", false, err) - recordPurchaseAudit(rec, req.CredentialScope, common.PurchaseResult{Error: err}, auditStatusError, false) + recordPurchaseAudit(rec, req.CredentialScope, common.PurchaseResult{Error: err}, auditStatusForError(err), false) // Full provider error text surfaces to the caller (feedback: // providers must never swallow the underlying SDK/HTTP error). return nil, fmt.Errorf("purchase commitment failed: %w", err) diff --git a/tools/purchase_test.go b/tools/purchase_test.go index 7030e85..70dc97e 100644 --- a/tools/purchase_test.go +++ b/tools/purchase_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "log" "os" "path/filepath" @@ -1268,3 +1269,60 @@ func TestRequireCredentialScopeGCPMessageIsActionable(t *testing.T) { "provider %q has an ambient fallback and must name it", tc.provider) } } + +// TestExecutePurchaseAuditStatusForOutcomeUnknown pins the audit status of a +// failed purchase: only an error wrapping common.ErrOutcomeUnknown is +// "unknown", matched by errors.Is and never by message text. Not parallel: it +// swaps the standard logger and the audit path. +func TestExecutePurchaseAuditStatusForOutcomeUnknown(t *testing.T) { + run := func(t *testing.T, fake *fakeServiceClient) (status, stderr string) { + t.Helper() + path := filepath.Join(t.TempDir(), "audit.jsonl") + t.Setenv(EnvAuditLog, path) + stderr = captureStandardLogger(t, func() { + _, _ = ExecutePurchase(context.Background(), PurchaseRequest{ + Region: "us-east-1", Recommendation: testRecommendation(), Confirm: true, CredentialScope: "scope", + ResolveClient: func(_ context.Context) (provider.ServiceClient, error) { return fake, nil }, + }) + }) + lines := readAuditLines(t, path) + require.Len(t, lines, 1) + var rec struct { + Status string `json:"status"` + } + require.NoError(t, json.Unmarshal([]byte(lines[0]), &rec)) + return rec.Status, stderr + } + + t.Run("a deep-wrapped sentinel returned as the error is unknown", func(t *testing.T) { + wrapped := fmt.Errorf("purchase failed: %w", fmt.Errorf("send: %w", common.ErrOutcomeUnknown)) + status, stderr := run(t, &fakeServiceClient{purchaseErr: wrapped}) + assert.Equal(t, auditStatusUnknown, status) + assert.Contains(t, stderr, "mcp purchase OUTCOME UNKNOWN") + }) + + t.Run("a sentinel in result.Error is unknown", func(t *testing.T) { + fake := &fakeServiceClient{purchaseResult: common.PurchaseResult{ + Error: fmt.Errorf("purchase response was empty: %w", common.ErrOutcomeUnknown), + }} + status, stderr := run(t, fake) + assert.Equal(t, auditStatusUnknown, status) + assert.Contains(t, stderr, "mcp purchase OUTCOME UNKNOWN") + }) + + t.Run("a plain error is error", func(t *testing.T) { + status, stderr := run(t, &fakeServiceClient{purchaseErr: errors.New("insufficient capacity")}) + assert.Equal(t, auditStatusError, status) + assert.Contains(t, stderr, "mcp purchase FAILED") + }) + + t.Run("the sentinel's text without the sentinel is error", func(t *testing.T) { + status, _ := run(t, &fakeServiceClient{purchaseErr: errors.New("purchase outcome unknown")}) + assert.Equal(t, auditStatusError, status, "the status must not depend on message text") + }) + + t.Run("a success is success", func(t *testing.T) { + status, _ := run(t, &fakeServiceClient{purchaseResult: common.PurchaseResult{Success: true, CommitmentID: "ri-1"}}) + assert.Equal(t, auditStatusSuccess, status) + }) +}