fix(api): map run-now failures by cause and stamp its audit fields

internal/api/execution_scope_test.go

@@ -212,7 +212,7 @@ func newScopeTestHandler(t *testing.T, exec *config.PurchaseExecution, scope []s
 	store.On("CancelExecutionAtomic", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(false, "", errScopeTestReached).Maybe()
 	store.On("CancelScheduledExecutionAtomic", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(false, "", errScopeTestReached).Maybe()
 	mockPurchase.On("ApproveAndExecute", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return("", errScopeTestReached).Maybe()
-	mockPurchase.On("RunPlannedPurchaseNow", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return("", errScopeTestReached).Maybe()
+	mockPurchase.On("RunPlannedPurchaseNow", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, "", errScopeTestReached).Maybe()
 	mockPurchase.On("CancelExecution", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Maybe()
 
 	return &Handler{auth: mockAuth, config: store, purchase: mockPurchase}

internal/api/handler_purchases.go

@@ -345,23 +345,42 @@ func (h *Handler) runPlannedPurchase(ctx context.Context, req *events.LambdaFunc
 		return nil, constraintErr
 	}
 
-	revocationToken, runErr := h.purchase.RunPlannedPurchaseNow(ctx, executionID, fourEyesActorIdentity(session), resolveCreatorUserID(session))
+	final, revocationToken, runErr := h.purchase.RunPlannedPurchaseNow(ctx, executionID, fourEyesActorIdentity(session), resolveCreatorUserID(session))
 	if runErr != nil {
-		return nil, NewClientError(409, fmt.Sprintf("execution %s cannot be started: %v", executionID, runErr))
+		return nil, runNowError(executionID, final, runErr)
 	}
 
 	// The shared execute funnel rotated the row's token into a revocation
 	// token whose raw value exists only here (issue #103); email it like the
 	// session-approve path does. Best-effort: the purchase already committed.
-	h.sendPurchaseExecutedEmail(ctx, req, execution, revocationToken, session.Email)
+	h.sendPurchaseExecutedEmail(ctx, req, final, revocationToken, session.Email)
 
 	return map[string]any{
 		"execution_id": executionID,
-		"status":       "completed",
+		"status":       final.Status,
 		"message":      "Purchase executed",
 	}, nil
 }
 
+// runNowError maps a RunPlannedPurchaseNow failure to its HTTP error. A non-nil
+// final means the purchase ran and money may have moved, so it is never a 409.
+// ErrNotFound and ErrExecutionNotInExpectedStatus are 409 only before a claim.
+func runNowError(executionID string, final *config.PurchaseExecution, err error) error {
+	switch {
+	case errors.Is(err, purchase.ErrFourEyesDenied):
+		return NewClientError(403, fmt.Sprintf("execution %s cannot be started: %v", executionID, err))
+	case final == nil && (errors.Is(err, config.ErrExecutionNotInExpectedStatus) || errors.Is(err, config.ErrNotFound)):
+		return NewClientError(409, fmt.Sprintf("execution %s cannot be started: %v", executionID, err))
+	case final == nil:
+		return fmt.Errorf("execution %s could not be started: %w", executionID, err)
+	case errors.Is(err, config.ErrAuditLoss):
+		return NewClientError(500, fmt.Sprintf("execution %s ran but its final status could not be saved: %v", executionID, err))
+	default:
+		return NewClientErrorWithDetails(502, fmt.Sprintf("execution %s failed: %v", executionID, err),
+			map[string]any{"execution_id": executionID, "status": final.Status})
+	}
+}
+
 // requireDeleteOrCancelPurchasePermission validates the request session and
 // returns it when the caller holds any of: delete:purchases (management),
 // update-any:purchases (full-scope management), cancel-any:purchases

internal/api/handler_purchases_run_now_integration_test.go

@@ -0,0 +1,144 @@
+//go:build integration
+
+package api
+
+import (
+	"context"
+	"errors"
+	"sync/atomic"
+	"testing"
+	"time"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/provider"
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/purchase"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+)
+
+// runNowService buys every recommendation except the call numbered failOn.
+type runNowService struct {
+	fanoutServiceClient
+	calls  atomic.Int32
+	failOn int32
+}
+
+func (s *runNowService) PurchaseCommitment(ctx context.Context, rec common.Recommendation, opts common.PurchaseOptions) (common.PurchaseResult, error) {
+	if s.calls.Add(1) == s.failOn {
+		return common.PurchaseResult{}, errors.New("provider rejected the purchase")
+	}
+	return s.fanoutServiceClient.PurchaseCommitment(ctx, rec, opts)
+}
+
+type runNowProvider struct {
+	fanoutProvider
+	service *runNowService
+}
+
+func (p *runNowProvider) GetServiceClient(context.Context, common.ServiceType, string) (provider.ServiceClient, error) {
+	return p.service, nil
+}
+
+type runNowFactory struct{ provider *runNowProvider }
+
+func (f runNowFactory) CreateAndValidateProvider(context.Context, string, *provider.ProviderConfig) (provider.Provider, error) {
+	return f.provider, nil
+}
+
+// newRunNowFixture is the pause-claim fixture (real Postgres store, pending
+// row on an auto-purchase plan) with the handler wired to a real manager whose
+// provider fails on purchase call failOn (0 = never), run by the row's creator
+// holding execute:purchases.
+func newRunNowFixture(t *testing.T, failOn int32, recs []config.RecommendationRecord) (*pauseClaimFixture, *runNowService) {
+	t.Helper()
+	f := newPauseClaimFixture(t)
+	f.execution.Recommendations = recs
+	require.NoError(t, f.store.SavePurchaseExecution(f.ctx, f.execution))
+	service := &runNowService{failOn: failOn}
+	mockAuth := new(MockAuthService)
+	session := &Session{UserID: *f.execution.CreatedByUserID, Email: "creator@example.com"}
+	mockAuth.On("ValidateSession", mock.Anything, "admin-token").Return(session, nil).Maybe()
+	mockAuth.grantAdminPurchaser()
+	f.handler.auth = mockAuth
+	f.handler.purchase = purchase.NewManager(purchase.ManagerConfig{
+		ConfigStore: f.store, EmailSender: &stubEmailNotifier{}, CredentialStore: &fanoutCredStore{},
+		ProviderFactory: runNowFactory{provider: &runNowProvider{service: service}},
+	})
+	return f, service
+}
+
+func (f *pauseClaimFixture) runNow() (any, error) {
+	return f.handler.runPlannedPurchase(f.ctx, f.request, f.execution.ExecutionID)
+}
+
+func twoRunNowRecs() []config.RecommendationRecord {
+	recs := append(fanoutRecs(), fanoutRecs()...)
+	recs[1].ResourceType = "m5.xlarge"
+	return recs
+}
+
+func TestRunNow_StampsExecutedAuditFieldsAndReturnsRowStatus(t *testing.T) {
+	f, service := newRunNowFixture(t, 0, fanoutRecs())
+	before := time.Now()
+	result, err := f.runNow()
+	require.NoError(t, err)
+
+	row, err := f.store.GetExecutionByID(f.ctx, f.execution.ExecutionID)
+	require.NoError(t, err)
+	assert.Equal(t, "completed", row.Status)
+	assert.Equal(t, row.Status, result.(map[string]any)["status"])
+	assert.EqualValues(t, 1, service.calls.Load())
+	require.NotNil(t, row.ExecutedAt, "run-now must stamp executed_at")
+	assert.WithinRange(t, *row.ExecutedAt, before.Add(-time.Second), time.Now())
+	require.NotNil(t, row.ExecutedByUserID, "run-now must stamp executed_by_user_id")
+	assert.Equal(t, *f.execution.CreatedByUserID, *row.ExecutedByUserID)
+	require.NotNil(t, row.PreApprovalSkipReason)
+	assert.Equal(t, "run-now", *row.PreApprovalSkipReason)
+}
+
+func TestRunNow_PartialFailureIsServerErrorWithRowStatus(t *testing.T) {
+	f, service := newRunNowFixture(t, 2, twoRunNowRecs())
+	result, err := f.runNow()
+	assert.Nil(t, result)
+
+	ce, ok := IsClientError(err)
+	require.True(t, ok, "expected a client-facing error, got %v", err)
+	assert.Equal(t, 502, ce.code, "money moved: this is an execution failure, not a 409 conflict")
+	row, getErr := f.store.GetExecutionByID(f.ctx, f.execution.ExecutionID)
+	require.NoError(t, getErr)
+	assert.Equal(t, "partially_completed", row.Status)
+	assert.Equal(t, row.Status, ce.Details()["status"])
+	assert.EqualValues(t, 2, service.calls.Load())
+	assert.NotNil(t, row.ExecutedAt)
+}
+
+func TestRunNow_FourEyesDenialIsForbiddenAndLeavesRowUntouched(t *testing.T) {
+	f, service := newRunNowFixture(t, 0, fanoutRecs())
+	require.NoError(t, f.store.SaveGlobalConfig(f.ctx, &config.GlobalConfig{RequireDifferentApprover: true}))
+
+	_, err := f.runNow()
+	ce, ok := IsClientError(err)
+	require.True(t, ok, "expected a client-facing error, got %v", err)
+	assert.Equal(t, 403, ce.code)
+	row, getErr := f.store.GetExecutionByID(f.ctx, f.execution.ExecutionID)
+	require.NoError(t, getErr)
+	assert.Equal(t, "pending", row.Status)
+	assert.Nil(t, row.ExecutedAt)
+	assert.Zero(t, service.calls.Load())
+}
+
+func TestRunNow_LostClaimIsConflict(t *testing.T) {
+	f, service := newRunNowFixture(t, 0, fanoutRecs())
+	f.execution.Status = "running"
+	require.NoError(t, f.store.SavePurchaseExecution(f.ctx, f.execution))
+
+	_, err := f.runNow()
+	assertPauseConflict(t, err)
+	row, getErr := f.store.GetExecutionByID(f.ctx, f.execution.ExecutionID)
+	require.NoError(t, getErr)
+	assert.Equal(t, "running", row.Status)
+	assert.Nil(t, row.ExecutedAt)
+	assert.Zero(t, service.calls.Load())
+}

internal/api/handler_purchases_test.go

@@ -1592,7 +1592,11 @@ func TestHandler_runPlannedPurchase(t *testing.T) {
 	// CAS-guarded funnel ApproveAndExecute uses (issue #218) rather than a
 	// bare TransitionExecutionStatus flip to "running" that no executor
 	// consumes.
-	mockPurchase.On("RunPlannedPurchaseNow", ctx, "11111111-1111-1111-1111-111111111111", "admin@example.com", mock.Anything).Return("raw-revocation-token", nil)
+	mockPurchase.On("RunPlannedPurchaseNow", ctx, "11111111-1111-1111-1111-111111111111", "admin@example.com", mock.Anything).Return(&config.PurchaseExecution{
+		ExecutionID:     "11111111-1111-1111-1111-111111111111",
+		Status:          "completed",
+		Recommendations: []config.RecommendationRecord{{Provider: "aws", Service: "ec2", Region: "us-east-1", UpfrontCost: 100}},
+	}, "raw-revocation-token", nil)
 	notify := "notify@example.com"
 	mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{NotificationEmail: &notify}, nil)
 
@@ -2183,7 +2187,7 @@ func TestHandler_runPlannedPurchase_NilExecution(t *testing.T) {
 	mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
 	mockAuth.grantAdmin()
 	mockPurchase.On("RunPlannedPurchaseNow", ctx, "99999999-9999-9999-9999-999999999999", "admin@example.com", mock.Anything).
-		Return("", fmt.Errorf("execution not found: 99999999-9999-9999-9999-999999999999"))
+		Return(nil, "", fmt.Errorf("execution not found: 99999999-9999-9999-9999-999999999999"))
 
 	handler := &Handler{purchase: mockPurchase, config: mockStore, auth: mockAuth}
 
@@ -2197,6 +2201,41 @@ func TestHandler_runPlannedPurchase_NilExecution(t *testing.T) {
 	assert.Nil(t, result)
 }
 
+func TestRunNowError(t *testing.T) {
+	ran := &config.PurchaseExecution{Status: "failed"}
+	cases := []struct {
+		name  string
+		final *config.PurchaseExecution
+		err   error
+		code  int // 0: not a client error, so the router answers a generic 500
+	}{
+		{"four-eyes denial", nil, fmt.Errorf("%w: same approver", purchase.ErrFourEyesDenied), 403},
+		{"lost claim", nil, fmt.Errorf("approve: %w", config.ErrExecutionNotInExpectedStatus), 409},
+		{"row gone", nil, fmt.Errorf("approve: %w", config.ErrNotFound), 409},
+		{"store error before claim", nil, errors.New("connection reset"), 0},
+		{"plan deleted after claim", ran, fmt.Errorf("failed to get plan: %w", config.ErrNotFound), 502},
+		{"audit loss wrapping not found", ran, fmt.Errorf("%w: %w", config.ErrAuditLoss, config.ErrNotFound), 500},
+		{"final status not saved", ran, fmt.Errorf("%w: boom", config.ErrAuditLoss), 500},
+		{"execution failed", ran, errors.New("provider rejected"), 502},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := runNowError("exec-1", tc.final, tc.err)
+			ce, ok := IsClientError(err)
+			if tc.code == 0 {
+				assert.False(t, ok)
+				assert.ErrorIs(t, err, tc.err)
+				return
+			}
+			require.True(t, ok)
+			assert.Equal(t, tc.code, ce.code)
+			if tc.code == 502 {
+				assert.Equal(t, "failed", ce.Details()["status"])
+			}
+		})
+	}
+}
+
 func TestHandler_deletePlannedPurchase_NilExecution(t *testing.T) {
 	ctx := context.Background()
 	mockStore := new(MockConfigStore)

internal/api/handler_test.go

@@ -1270,7 +1270,7 @@ func TestHandler_HandleRequest_RunPlannedPurchase(t *testing.T) {
 	// resolves to the stateless admin-api-key principal rather than the
 	// bearer-token session, so the actor identity fourEyesActorIdentity
 	// derives is the sentinel, not the session's email.
-	mockPurchase.On("RunPlannedPurchaseNow", mock.Anything, "11111111-1111-1111-1111-111111111111", "admin-api-key", mock.Anything).Return("", nil)
+	mockPurchase.On("RunPlannedPurchaseNow", mock.Anything, "11111111-1111-1111-1111-111111111111", "admin-api-key", mock.Anything).Return(&config.PurchaseExecution{ExecutionID: "11111111-1111-1111-1111-111111111111", Status: "completed"}, "", nil)
 
 	handler := &Handler{purchase: mockPurchase, config: mockStore, auth: mockAuth, corsAllowedOrigin: "*", apiKey: "test-key"}
 

internal/api/mocks_test.go

@@ -58,9 +58,10 @@ func (m *MockPurchaseManager) ApproveAndExecute(ctx context.Context, execID, act
 	return args.String(0), args.Error(1)
 }
 
-func (m *MockPurchaseManager) RunPlannedPurchaseNow(ctx context.Context, execID, actor string, transitionedBy *string) (string, error) {
+func (m *MockPurchaseManager) RunPlannedPurchaseNow(ctx context.Context, execID, actor string, transitionedBy *string) (*config.PurchaseExecution, string, error) {
 	args := m.Called(ctx, execID, actor, transitionedBy)
-	return args.String(0), args.Error(1)
+	exec, _ := args.Get(0).(*config.PurchaseExecution)
+	return exec, args.String(1), args.Error(2)
 }
 
 func (m *MockPurchaseManager) CancelExecution(ctx context.Context, execID, token, actor string) error {

internal/api/types.go

@@ -146,7 +146,7 @@ type PurchaseManagerInterface interface {
 	// execute immediately (the "Run now" button), sharing ApproveAndExecute's
 	// 4-eyes-gated, CAS-guarded funnel instead of a bare status flip that
 	// nothing else consumes (issue #218).
-	RunPlannedPurchaseNow(ctx context.Context, execID, actor string, transitionedBy *string) (string, error)
+	RunPlannedPurchaseNow(ctx context.Context, execID, actor string, transitionedBy *string) (*config.PurchaseExecution, string, error)
 	CancelExecution(ctx context.Context, execID, token, actor string) error
 }
 

internal/config/types.go

@@ -355,17 +355,17 @@ type PurchaseExecution struct {
 	// always carry a non-nil value.
 	ApprovalTokenExpiresAt *time.Time `json:"approval_token_expires_at,omitempty" dynamodbav:"approval_token_expires_at,omitempty"`
 	// ExecutedByUserID is the UUID of the session user who triggered a
-	// direct-execute (issue #289, execute-any/execute-own). NULL on rows
-	// that went through the normal approval flow. Non-null signals the
-	// approval step was intentionally skipped by an authorized operator.
+	// direct-execute (issue #289, execute-any/execute-own) or a run-now. NULL on rows
+	// that went through the normal approval flow. Together with
+	// pre_approval_skip_reason it marks a skipped approval email/wait (see ApprovedBy).
 	// Migration 000058 adds the column.
 	ExecutedByUserID *string `json:"executed_by_user_id,omitempty" dynamodbav:"executed_by_user_id,omitempty"`
-	// ExecutedAt is the UTC timestamp when the direct-execute path fired.
+	// ExecutedAt is the UTC timestamp when the direct-execute or run-now path fired.
 	// NULL for rows on the normal approval flow. Migration 000058.
 	ExecutedAt *time.Time `json:"executed_at,omitempty" dynamodbav:"executed_at,omitempty"`
 	// PreApprovalSkipReason is a human-readable token describing why the
-	// approval step was skipped. For direct-execute rows it is the literal
-	// string "direct-execute permission". NULL on every normal-flow row.
+	// approval email/scheduled wait was skipped: "direct-execute permission" or
+	// "run-now". NULL on every normal-flow row.
 	// Migration 000058.
 	PreApprovalSkipReason *string `json:"pre_approval_skip_reason,omitempty" dynamodbav:"pre_approval_skip_reason,omitempty"`
 	// IdempotencyKey is the stable lineage anchor the per-rec provider