fix(server): stop reporting fabricated success on three operator paths

internal/server/app_test.go

@@ -78,7 +78,8 @@ func TestHttpToLambdaRequest_XForwardedFor(t *testing.T) {
 	req.Header.Set("X-Forwarded-For", "1.2.3.4, 5.6.7.8")
 	req.Header.Set("User-Agent", "TestAgent/1.0")
 
-	lambdaReq := httpToLambdaRequest(req)
+	lambdaReq, err := httpToLambdaRequest(httptest.NewRecorder(), req)
+	testutil.AssertNoError(t, err)
 
 	testutil.AssertEqual(t, "5.6.7.8", lambdaReq.RequestContext.HTTP.SourceIP)
 	testutil.AssertEqual(t, "TestAgent/1.0", lambdaReq.RequestContext.HTTP.UserAgent)
@@ -88,7 +89,8 @@ func TestHttpToLambdaRequest_NilBody(t *testing.T) {
 	req := httptest.NewRequestWithContext(context.Background(), "GET", "/api/test", nil)
 	req.Body = nil
 
-	lambdaReq := httpToLambdaRequest(req)
+	lambdaReq, err := httpToLambdaRequest(httptest.NewRecorder(), req)
+	testutil.AssertNoError(t, err)
 
 	testutil.AssertEqual(t, "", lambdaReq.Body)
 }

internal/server/handler.go

@@ -287,8 +287,9 @@ func (app *Application) handleSendNotifications(ctx context.Context) (*purchase.
 func (app *Application) handleCleanupExpiredRecords(ctx context.Context) (map[string]int64, error) {
 	log.Println("Cleaning up expired records...")
 
+	// No sessions_deleted counter: CleanupExpiredSessions reports no count,
+	// so it could only ever be a fabricated 0 (#326).
 	result := map[string]int64{
-		"sessions_deleted":   0,
 		"executions_deleted": 0,
 	}
 
@@ -316,7 +317,7 @@ func (app *Application) handleCleanupExpiredRecords(ctx context.Context) (map[st
 		}
 	}
 
-	log.Printf("Cleanup complete: %d sessions, %d executions deleted", result["sessions_deleted"], result["executions_deleted"])
+	log.Printf("Cleanup complete: %d executions deleted", result["executions_deleted"])
 	return result, nil
 }
 
@@ -385,11 +386,11 @@ func (app *Application) handleFinalizeRevocations(ctx context.Context) (*purchas
 func (app *Application) handleRefreshAnalytics(ctx context.Context) (map[string]any, error) {
 	log.Println("Refreshing analytics...")
 
+	// No partition counters: this task never creates or drops partitions
+	// (handleCollectAnalytics does), so they could only be fabricated zeros.
 	result := map[string]any{
-		"status":             "success",
-		"views_refreshed":    0,
-		"partitions_created": 0,
-		"partitions_dropped": 0,
+		"status":          "success",
+		"views_refreshed": 0,
 	}
 
 	// Refresh materialized views if analytics store is available.
@@ -406,6 +407,9 @@ func (app *Application) handleRefreshAnalytics(ctx context.Context) (map[string]
 			log.Println("Materialized views refreshed successfully")
 		}
 	} else {
+		// Nothing ran, so this is not a success; handleCollectAnalytics
+		// reports the same condition as "skipped" (#326).
+		result["status"] = "skipped"
 		log.Println("Analytics store not available, skipping materialized view refresh")
 	}
 

internal/server/handler_coverage_test.go

@@ -31,8 +31,12 @@ func TestHandleCleanupExpiredRecords_WithAuthAndConfig(t *testing.T) {
 	result, err := app.handleCleanupExpiredRecords(ctx)
 	testutil.AssertNoError(t, err)
 	testutil.AssertTrue(t, result != nil, "expected non-nil result map")
+	// CleanupExpiredSessions reports no count, so the result must not carry a
+	// sessions_deleted counter that is always 0 (#326, A06-018).
 	_, hasSessionsKey := result["sessions_deleted"]
-	testutil.AssertTrue(t, hasSessionsKey, "expected sessions_deleted key")
+	testutil.AssertTrue(t, !hasSessionsKey, "sessions_deleted is never populated and must not be reported")
+	_, hasExecutionsKey := result["executions_deleted"]
+	testutil.AssertTrue(t, hasExecutionsKey, "expected executions_deleted key")
 }
 
 func TestHandleCleanupExpiredRecords_NilAuthAndConfig(t *testing.T) {
@@ -97,6 +101,19 @@ func TestHandleRefreshAnalytics_Success(t *testing.T) {
 	result, err := app.handleRefreshAnalytics(ctx)
 	testutil.AssertNoError(t, err)
 	testutil.AssertEqual(t, "success", result["status"])
+	testutil.AssertEqual(t, 1, result["views_refreshed"])
+	assertNoPartitionCounters(t, result)
+}
+
+// assertNoPartitionCounters checks that handleRefreshAnalytics does not report
+// partition counters: it never creates or drops partitions, so they could only
+// ever be fabricated zeros (#326, A06-018).
+func assertNoPartitionCounters(t *testing.T, result map[string]any) {
+	t.Helper()
+	for _, key := range []string{"partitions_created", "partitions_dropped"} {
+		_, ok := result[key]
+		testutil.AssertTrue(t, !ok, key+" is never populated and must not be reported")
+	}
 }
 
 func TestHandleRefreshAnalytics_RefreshError(t *testing.T) {
@@ -117,6 +134,11 @@ func TestHandleRefreshAnalytics_NilAnalytics(t *testing.T) {
 	result, err := app.handleRefreshAnalytics(ctx)
 	testutil.AssertNoError(t, err)
 	testutil.AssertTrue(t, result != nil, "expected non-nil result")
+	// Nothing was refreshed, so the run must not report success (#326,
+	// A06-018); handleCollectAnalytics reports "skipped" for the same case.
+	testutil.AssertEqual(t, "skipped", result["status"])
+	testutil.AssertEqual(t, 0, result["views_refreshed"])
+	assertNoPartitionCounters(t, result)
 }
 
 // ----- configExchangeStoreAdapter — CompleteRIExchange / FailRIExchange -----

internal/server/handler_ladder.go

@@ -142,9 +142,8 @@ func (app *Application) resolveLadderAccount(ctx context.Context) (accountID, re
 }
 
 // defaultLadderAccountResolver resolves the Lambda's own AWS account ID via STS
-// STRICTLY: unlike resolveAccountID (which returns the "unknown" sentinel for
-// audit-only use in the RI-exchange path), this returns an error when the
-// account cannot be determined. The ladder path uses the account ID to SCOPE
+// STRICTLY, like resolveAccountID in the RI-exchange path: this returns an
+// error when the account cannot be determined. The ladder path uses the account ID to SCOPE
 // which configs run, so a fabricated/"unknown" value is unsafe: it would skip
 // every config as multi-account and report a false success.
 func defaultLadderAccountResolver(ctx context.Context) (accountID, region string, err error) {

internal/server/handler_ri_exchange.go

@@ -2,11 +2,11 @@ package server
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"log"
 	"time"
 
-	"github.com/aws/aws-sdk-go-v2/aws"
 	awsconfig "github.com/aws/aws-sdk-go-v2/config"
 	"github.com/aws/aws-sdk-go-v2/service/sts"
 
@@ -52,6 +52,10 @@ func (app *Application) handleRIExchangeReshape(ctx context.Context) (*exchange.
 	if err != nil {
 		return nil, fmt.Errorf("failed to load AWS config: %w", err)
 	}
+	accountID, err := resolveAccountID(ctx, sts.NewFromConfig(awsCfg))
+	if err != nil {
+		return nil, fmt.Errorf("RI exchange reshape aborted: %w", err)
+	}
 
 	ec2Client := awsprovider.NewEC2ClientDirect(awsCfg)
 	clients := riExchangeClients{
@@ -64,7 +68,7 @@ func (app *Application) handleRIExchangeReshape(ctx context.Context) (*exchange.
 		},
 		exchangeClient: exchange.NewExchangeClient(awsCfg),
 		lookupOffering: newConvertibleOfferingLookup(ec2Client),
-		accountID:      resolveAccountID(ctx, awsCfg),
+		accountID:      accountID,
 		region:         awsCfg.Region,
 	}
 
@@ -127,6 +131,11 @@ func newConvertibleOfferingLookup(finder convertibleOfferingFinder) exchange.Loo
 // executeRIExchangeReshape contains the core exchange logic, separated from AWS
 // client construction for testability.
 func (app *Application) executeRIExchangeReshape(ctx context.Context, cfg *config.GlobalConfig, clients riExchangeClients) (*exchange.AutoExchangeResult, error) {
+	// Every record the run creates carries this account ID for the audit
+	// trail, so a run without one stops before touching AWS or the store.
+	if clients.accountID == "" {
+		return nil, errors.New("no AWS account ID resolved for the RI exchange audit trail")
+	}
 	instances, err := clients.listConvertibleRIs(ctx)
 	if err != nil {
 		return nil, fmt.Errorf("failed to list convertible RIs: %w", err)
@@ -174,19 +183,26 @@ func (app *Application) executeRIExchangeReshape(ctx context.Context, cfg *confi
 	return result, nil
 }
 
-// resolveAccountID fetches the AWS account ID via STS. Returns "unknown" on failure
-// since account_id is stored for audit trail only and is not used to scope queries.
-func resolveAccountID(ctx context.Context, awsCfg aws.Config) string {
-	stsClient := sts.NewFromConfig(awsCfg)
-	identity, err := stsClient.GetCallerIdentity(ctx, &sts.GetCallerIdentityInput{})
+// callerIdentityGetter is the slice of the STS client resolveAccountID needs.
+// Satisfied by *sts.Client.
+type callerIdentityGetter interface {
+	GetCallerIdentity(ctx context.Context, params *sts.GetCallerIdentityInput, optFns ...func(*sts.Options)) (*sts.GetCallerIdentityOutput, error)
+}
+
+// resolveAccountID fetches the caller's AWS account ID via STS and fails when
+// it cannot be determined. The ID is written onto every RI exchange record the
+// run creates, so a placeholder such as "unknown" would put a fabricated
+// account on the audit trail of a money action (#326); the run is aborted
+// instead, as defaultLadderAccountResolver does.
+func resolveAccountID(ctx context.Context, client callerIdentityGetter) (string, error) {
+	identity, err := client.GetCallerIdentity(ctx, &sts.GetCallerIdentityInput{})
 	if err != nil {
-		log.Printf("Warning: failed to get AWS account ID via STS: %v (using 'unknown')", err)
-		return "unknown"
+		return "", fmt.Errorf("resolve caller AWS account via STS: %w", err)
 	}
-	if identity.Account != nil {
-		return *identity.Account
+	if identity == nil || identity.Account == nil || *identity.Account == "" {
+		return "", errors.New("STS GetCallerIdentity returned no account ID")
 	}
-	return "unknown"
+	return *identity.Account, nil
 }
 
 // sendExchangeNotification sends email notifications based on the exchange result.

internal/server/handler_ri_exchange_account_test.go

@@ -0,0 +1,101 @@
+package server
+
+import (
+	"context"
+	"errors"
+	"strings"
+	"testing"
+
+	"github.com/aws/aws-sdk-go-v2/aws"
+	"github.com/aws/aws-sdk-go-v2/service/sts"
+
+	"github.com/LeanerCloud/cloud-commitments-go/providers/aws/recommendations"
+	ec2svc "github.com/LeanerCloud/cloud-commitments-go/providers/aws/services/ec2"
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/testutil"
+)
+
+// fakeCallerIdentity is a canned STS GetCallerIdentity response.
+type fakeCallerIdentity struct {
+	out *sts.GetCallerIdentityOutput
+	err error
+}
+
+func (f fakeCallerIdentity) GetCallerIdentity(context.Context, *sts.GetCallerIdentityInput, ...func(*sts.Options)) (*sts.GetCallerIdentityOutput, error) {
+	return f.out, f.err
+}
+
+// resolveAccountID must fail instead of inventing an "unknown" account ID,
+// which used to be written onto every RI exchange audit row the run created
+// (#326, A06-025).
+func TestResolveAccountID_RefusesToFabricateAnAccount(t *testing.T) {
+	cases := map[string]fakeCallerIdentity{
+		"STS error":     {err: errors.New("ExpiredToken: the security token included in the request is expired")},
+		"nil output":    {},
+		"nil account":   {out: &sts.GetCallerIdentityOutput{}},
+		"empty account": {out: &sts.GetCallerIdentityOutput{Account: aws.String("")}},
+	}
+	for name, fake := range cases {
+		t.Run(name, func(t *testing.T) {
+			accountID, err := resolveAccountID(context.Background(), fake)
+
+			testutil.AssertError(t, err)
+			testutil.AssertEqual(t, "", accountID)
+		})
+	}
+}
+
+func TestResolveAccountID_ReturnsTheCallerAccount(t *testing.T) {
+	fake := fakeCallerIdentity{out: &sts.GetCallerIdentityOutput{Account: aws.String("123456789012")}}
+
+	accountID, err := resolveAccountID(context.Background(), fake)
+
+	testutil.AssertNoError(t, err)
+	testutil.AssertEqual(t, "123456789012", accountID)
+}
+
+// A reshape run without a resolved account ID must stop before it lists RIs
+// or writes any exchange record, so no audit row carries a blank account.
+func TestExecuteRIExchangeReshape_RefusesEmptyAccountID(t *testing.T) {
+	ctx := testutil.TestContext(t)
+	writes := 0
+	store := &mockConfigStoreForExchange{
+		saveRIExchangeRecordFunc: func(context.Context, *config.RIExchangeRecord) error {
+			writes++
+			return nil
+		},
+		reserveRIExchangeFunc: func(context.Context, *config.RIExchangeRecord, string, string) (string, error) {
+			writes++
+			return "", errors.New("unexpected reservation")
+		},
+	}
+	app := &Application{Config: store}
+	cfg := &config.GlobalConfig{
+		RIExchangeEnabled:              true,
+		RIExchangeMode:                 "auto",
+		RIExchangeUtilizationThreshold: 95.0,
+		RIExchangeLookbackDays:         30,
+	}
+
+	listed := false
+	clients := riExchangeClients{
+		listConvertibleRIs: func(context.Context) ([]ec2svc.ConvertibleRI, error) {
+			listed = true
+			return nil, nil
+		},
+		getRIUtilization: func(context.Context, int) ([]recommendations.RIUtilization, error) {
+			return nil, nil
+		},
+		exchangeClient: &mockExchangeClient{},
+		accountID:      "",
+		region:         "us-east-1",
+	}
+
+	result, err := app.executeRIExchangeReshape(ctx, cfg, clients)
+
+	testutil.AssertError(t, err)
+	testutil.AssertTrue(t, strings.Contains(err.Error(), "account ID"), "expected an account ID error, got: "+err.Error())
+	testutil.AssertTrue(t, result == nil, "expected no result")
+	testutil.AssertTrue(t, !listed, "convertible RIs must not be listed without an account ID")
+	testutil.AssertEqual(t, 0, writes)
+}

internal/server/handler_ri_exchange_test.go

@@ -489,6 +489,7 @@ func TestExecuteRIExchangeReshape_ListRIsFailure(t *testing.T) {
 		listConvertibleRIs: func(ctx context.Context) ([]ec2svc.ConvertibleRI, error) {
 			return nil, errors.New("EC2 API throttled")
 		},
+		accountID: "123456789",
 	}
 
 	_, err := app.executeRIExchangeReshape(ctx, cfg, clients)
@@ -517,6 +518,7 @@ func TestExecuteRIExchangeReshape_UtilizationFailure(t *testing.T) {
 		getRIUtilization: func(ctx context.Context, lookbackDays int) ([]recommendations.RIUtilization, error) {
 			return nil, errors.New("Cost Explorer unavailable")
 		},
+		accountID: "123456789",
 	}
 
 	_, err := app.executeRIExchangeReshape(ctx, cfg, clients)