fix(plans): guard delete, reschedule and resume of managed plans

internal/api/handler_plans.go

@@ -434,6 +434,10 @@ func (h *Handler) deletePlan(ctx context.Context, req *events.LambdaFunctionURLR
 	}
 
 	if err := h.config.DeletePurchasePlan(ctx, planID); err != nil {
+		if errors.Is(err, config.ErrManagedPlanLiveExecutions) {
+			return nil, planEditorRefusal(http.StatusConflict, codeManagedPlanLiveExecutions,
+				"this plan still has active or unsettled purchases; cancel or revoke them first")
+		}
 		return nil, err
 	}
 

internal/api/handler_plans_managed_add_accounts_integration_test.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/LeanerCloud/cloud-commitments-platform/internal/auth"
 	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
+	"github.com/jackc/pgx/v5"
 	"github.com/stretchr/testify/require"
 )
 
@@ -44,6 +45,27 @@ func (f *createConcurrencyFixture) extraAccount(ctx context.Context, t *testing.
 	return acct.ID
 }
 
+// holdPlanLock stands in for the editor command: an open transaction holding
+// the plan row FOR UPDATE, rolled back at test end unless the test commits it.
+func (f *createConcurrencyFixture) holdPlanLock(ctx context.Context, t *testing.T) pgx.Tx {
+	t.Helper()
+	command, err := f.pool.Begin(ctx)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = command.Rollback(ctx) })
+	_, err = command.Exec(ctx, `SELECT 1 FROM purchase_plans WHERE id = $1 FOR UPDATE`, f.planID)
+	require.NoError(t, err)
+	return command
+}
+
+func awaitLockWait(ctx context.Context, t *testing.T, f *createConcurrencyFixture) {
+	t.Helper()
+	require.Eventually(t, func() bool {
+		var n int
+		require.NoError(t, f.pool.QueryRow(ctx, `SELECT count(*) FROM pg_stat_activity WHERE wait_event_type = 'Lock'`).Scan(&n))
+		return n > 0
+	}, 15*time.Second, 50*time.Millisecond, "writer must wait on the plan lock")
+}
+
 const (
 	execCount = `SELECT count(*) FROM purchase_executions WHERE plan_id = $1`
 	acctCount = `SELECT count(*) FROM plan_accounts WHERE plan_id = $1`
@@ -130,21 +152,12 @@ func TestManagedMarkerCommittedUnderLockRefusesWaitingWriters(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			f := newCreateConcurrencyFixture(ctx, t)
 			o := other(f)
-			command, err := f.pool.Begin(ctx)
-			require.NoError(t, err)
-			defer func() { _ = command.Rollback(ctx) }()
-			_, err = command.Exec(ctx, `SELECT 1 FROM purchase_plans WHERE id = $1 FOR UPDATE`, f.planID)
-			require.NoError(t, err)
-
+			command := f.holdPlanLock(ctx, t)
 			done := make(chan error, 1)
 			go func() { done <- write(f, o) }()
-			require.Eventually(t, func() bool {
-				var n int
-				require.NoError(t, f.pool.QueryRow(ctx, `SELECT count(*) FROM pg_stat_activity WHERE wait_event_type = 'Lock'`).Scan(&n))
-				return n > 0
-			}, 15*time.Second, 50*time.Millisecond, "writer must wait on the plan lock")
+			awaitLockWait(ctx, t, f)
 
-			_, err = command.Exec(ctx, `UPDATE purchase_plans SET ramp_schedule = ramp_schedule || '{"dates_managed":true}' WHERE id = $1`, f.planID)
+			_, err := command.Exec(ctx, `UPDATE purchase_plans SET ramp_schedule = ramp_schedule || '{"dates_managed":true}' WHERE id = $1`, f.planID)
 			require.NoError(t, err)
 			require.NoError(t, command.Commit(ctx))
 			requireEditorRefusal(t, <-done)

internal/api/handler_plans_managed_delete_resume_integration_test.go

@@ -0,0 +1,181 @@
+//go:build integration
+
+package api
+
+import (
+	"context"
+	"errors"
+	"net/http"
+	"testing"
+	"time"
+
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/auth"
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
+	"github.com/google/uuid"
+	"github.com/stretchr/testify/require"
+)
+
+func (f *createConcurrencyFixture) saveRow(ctx context.Context, t *testing.T, status string, noSubmission bool, scheduled time.Time, recs bool) string {
+	t.Helper()
+	row := &config.PurchaseExecution{ExecutionID: uuid.NewString(), PlanID: f.planID, Status: status, StepNumber: 3, ScheduledDate: scheduled, NoSubmission: noSubmission}
+	if recs {
+		row.Recommendations = []config.RecommendationRecord{{ID: "r1", Provider: "azure", Service: "compute", Selected: true}}
+	}
+	require.NoError(t, f.store.SavePurchaseExecution(ctx, row))
+	return row.ExecutionID
+}
+
+func (f *createConcurrencyFixture) deletePlan(ctx context.Context, h *Handler) error {
+	_, err := h.deletePlan(ctx, managedGuardRequest(""), f.planID)
+	return err
+}
+
+func requireLiveExecutionsRefusal(t *testing.T, err error) {
+	t.Helper()
+	var ce *clientError
+	require.True(t, errors.As(err, &ce), "want a client error, got %v", err)
+	require.Equal(t, http.StatusConflict, ce.code)
+	require.Equal(t, codeManagedPlanLiveExecutions, ce.details["code"])
+}
+
+func TestDeleteManagedPlanByExecutionStatus(t *testing.T) {
+	ctx := context.Background()
+	cases := []struct {
+		status       string
+		noSubmission bool
+		blocks       bool
+	}{
+		{"pending", false, true}, {"notified", false, true}, {"scheduled", false, true}, {"approved", false, true},
+		{"running", false, true}, {"paused", false, true}, {"failed", false, true},
+		{"completed", false, false}, {"partially_completed", false, false}, {"canceled", false, false},
+		{"expired", false, false}, {"failed", true, false},
+	}
+	for _, tc := range cases {
+		name := tc.status
+		if tc.noSubmission {
+			name += "_no_submission"
+		}
+		t.Run(name, func(t *testing.T) {
+			f := newCreateConcurrencyFixture(ctx, t)
+			setPlanManaged(ctx, t, f, true)
+			id := f.saveRow(ctx, t, tc.status, tc.noSubmission, time.Now(), false)
+			err := f.deletePlan(ctx, f.handler)
+			var planID *string
+			require.NoError(t, f.pool.QueryRow(ctx, `SELECT plan_id::text FROM purchase_executions WHERE execution_id = $1`, id).Scan(&planID))
+			if tc.blocks {
+				requireLiveExecutionsRefusal(t, err)
+				require.NotNil(t, planID, "a refused delete must not detach the root")
+				require.Equal(t, 1, f.count(ctx, t, `SELECT count(*) FROM purchase_plans WHERE id = $1`))
+				return
+			}
+			require.NoError(t, err)
+			require.Nil(t, planID, "settled history detaches as before")
+		})
+	}
+	t.Run("unmanaged plan with a live row deletes as before", func(t *testing.T) {
+		f := newCreateConcurrencyFixture(ctx, t)
+		f.saveRow(ctx, t, "pending", false, time.Now(), false)
+		require.NoError(t, f.deletePlan(ctx, f.handler))
+	})
+}
+
+func TestDeleteManagedPlanWaitsForCommandAndSeesItsRoot(t *testing.T) {
+	ctx := context.Background()
+	f := newCreateConcurrencyFixture(ctx, t)
+	command := f.holdPlanLock(ctx, t)
+	done := make(chan error, 1)
+	go func() { done <- f.deletePlan(ctx, f.handler) }()
+	awaitLockWait(ctx, t, f)
+
+	id := uuid.NewString()
+	_, err := command.Exec(ctx, `INSERT INTO purchase_executions (execution_id, plan_id, status, step_number, scheduled_date)
+		VALUES ($1, $2, 'pending', 1, now())`, id, f.planID)
+	require.NoError(t, err)
+	_, err = command.Exec(ctx, `UPDATE purchase_plans SET ramp_schedule = ramp_schedule || '{"dates_managed":true}' WHERE id = $1`, f.planID)
+	require.NoError(t, err)
+	require.NoError(t, command.Commit(ctx))
+
+	requireLiveExecutionsRefusal(t, <-done)
+	require.Equal(t, 1, f.count(ctx, t, `SELECT count(*) FROM purchase_executions WHERE plan_id = $1`))
+}
+
+func TestRescheduleAwaitingExecutionOnManagedPlan(t *testing.T) {
+	ctx := context.Background()
+	f := newCreateConcurrencyFixture(ctx, t)
+	orig := time.Now().Add(-time.Hour).UTC().Truncate(time.Second)
+	id := f.saveRow(ctx, t, "pending", false, orig, true)
+	moved := time.Now().Add(48 * time.Hour).UTC().Truncate(time.Second)
+
+	ok, err := f.store.RescheduleAwaitingExecution(ctx, id, moved)
+	require.NoError(t, err)
+	require.True(t, ok, "unmanaged plan reschedules as before")
+
+	setPlanManaged(ctx, t, f, true)
+	ok, err = f.store.RescheduleAwaitingExecution(ctx, id, orig)
+	require.ErrorIs(t, err, config.ErrEditorReconciliationRequired)
+	require.False(t, ok)
+	row, err := f.store.GetExecutionByID(ctx, id)
+	require.NoError(t, err)
+	require.True(t, row.ScheduledDate.Equal(moved), "refused reschedule leaves the date")
+}
+
+func TestRescheduleWaitsForCommandThenRefuses(t *testing.T) {
+	ctx := context.Background()
+	f := newCreateConcurrencyFixture(ctx, t)
+	orig := time.Now().Add(-time.Hour).UTC().Truncate(time.Second)
+	id := f.saveRow(ctx, t, "pending", false, orig, true)
+	command := f.holdPlanLock(ctx, t)
+	done := make(chan error, 1)
+	go func() {
+		_, err := f.store.RescheduleAwaitingExecution(ctx, id, time.Now().Add(48*time.Hour))
+		done <- err
+	}()
+	awaitLockWait(ctx, t, f)
+	_, err := command.Exec(ctx, `UPDATE purchase_plans SET ramp_schedule = ramp_schedule || '{"dates_managed":true}' WHERE id = $1`, f.planID)
+	require.NoError(t, err)
+	require.NoError(t, command.Commit(ctx))
+
+	require.ErrorIs(t, <-done, config.ErrEditorReconciliationRequired)
+	row, err := f.store.GetExecutionByID(ctx, id)
+	require.NoError(t, err)
+	require.True(t, row.ScheduledDate.Equal(orig))
+}
+
+func TestResumeOnManagedPlanRefusesBeforeTransition(t *testing.T) {
+	ctx := context.Background()
+	f := newCreateConcurrencyFixture(ctx, t)
+	setPlanManaged(ctx, t, f, true)
+	id := f.saveRow(ctx, t, "paused", false, time.Now().Add(-time.Hour), true)
+
+	_, err := f.handler.resumePlannedPurchase(ctx, managedGuardRequest(""), id)
+	requireEditorRefusal(t, err)
+	row, err := f.store.GetExecutionByID(ctx, id)
+	require.NoError(t, err)
+	require.Equal(t, "paused", row.Status)
+}
+
+func TestDeleteAndResumeAuthorizationPrecedeManagedGuard(t *testing.T) {
+	ctx := context.Background()
+	f := newCreateConcurrencyFixture(ctx, t)
+	setPlanManaged(ctx, t, f, true)
+	id := f.saveRow(ctx, t, "paused", false, time.Now().Add(-time.Hour), true)
+	f.saveRow(ctx, t, "pending", false, time.Now(), false)
+	view := []auth.Permission{{Action: auth.ActionView, Resource: auth.ResourcePlans}}
+	admin := []auth.Permission{{Action: auth.ActionAdmin, Resource: auth.ResourceAll}}
+
+	forbidden := limitedHandler(f, view, nil)
+	scoped := limitedHandler(f, admin, []string{"unrelated-account"})
+	_, resumeErr := forbidden.resumePlannedPurchase(ctx, managedGuardRequest(""), id)
+	for name, got := range map[string]error{"delete": f.deletePlan(ctx, forbidden), "resume": resumeErr} {
+		var ce *clientError
+		require.True(t, errors.As(got, &ce), "%s: %v", name, got)
+		require.Equal(t, http.StatusForbidden, ce.code, name)
+	}
+	_, scopedResumeErr := scoped.resumePlannedPurchase(ctx, managedGuardRequest(""), id)
+	for name, got := range map[string]error{"delete": f.deletePlan(ctx, scoped), "resume": scopedResumeErr} {
+		require.True(t, IsNotFoundError(got), "scoped %s must look like not found, got %v", name, got)
+	}
+	row, err := f.store.GetExecutionByID(ctx, id)
+	require.NoError(t, err)
+	require.Equal(t, "paused", row.Status)
+}

internal/api/handler_purchases.go

@@ -278,6 +278,9 @@ func (h *Handler) requireResumeVetoWindow(ctx context.Context, executionID strin
 	if err != nil {
 		return fmt.Errorf("failed to read plan %s before resuming execution %s: %w", row.PlanID, executionID, err)
 	}
+	if plan.RampSchedule.DatesManaged {
+		return editorReconciliationError()
+	}
 	if vetoErr := plan.RequireVetoWindow(); vetoErr != nil {
 		return NewClientError(409, fmt.Sprintf("execution %s cannot be resumed: %v", executionID, vetoErr))
 	}
@@ -306,6 +309,9 @@ func (h *Handler) pushPastScheduledDate(ctx context.Context, row *config.Purchas
 	// approve landing after the resume must not be overwritten. A lost
 	// condition is a benign no-op (the row is no longer awaiting approval).
 	if _, err := h.config.RescheduleAwaitingExecution(ctx, row.ExecutionID, now.AddDate(0, 0, plan.NotificationDaysBefore)); err != nil {
+		if errors.Is(err, config.ErrEditorReconciliationRequired) {
+			return editorReconciliationError()
+		}
 		return fmt.Errorf("failed to reschedule resumed execution %s: %w", row.ExecutionID, err)
 	}
 	return nil

internal/api/openapi.yaml

@@ -362,11 +362,11 @@ paths:
         '404':
           $ref: '#/components/responses/NotFound'
         '409':
-          description: The plan changed concurrently, or the request changes the ramp schedule of a plan that has already completed a step. Keep the schedule, or create a new plan.
+          description: The plan changed concurrently, the request changes the ramp schedule of a plan that has already completed a step (keep the schedule, or create a new plan), or the plan is managed by the schedule editor and the request changes what only the editor may (code editor_reconciliation_required).
           content:
             application/json:
               schema:
-                $ref: '#/components/schemas/Error'
+                $ref: '#/components/schemas/ErrorWithCode'
     patch:
       operationId: patchPlan
       tags: [Plans]
@@ -394,6 +394,8 @@ paths:
           $ref: '#/components/responses/Forbidden'
         '404':
           $ref: '#/components/responses/NotFound'
+        '409':
+          $ref: '#/components/responses/EditorReconciliationRequired'
     delete:
       operationId: deletePlan
       tags: [Plans]
@@ -413,6 +415,12 @@ paths:
           $ref: '#/components/responses/Forbidden'
         '404':
           $ref: '#/components/responses/NotFound'
+        '409':
+          description: The plan is managed by the schedule editor and still has active or unsettled purchases. Cancel or revoke them first (code managed_plan_has_live_executions).
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/ErrorWithCode'
 
   /api/plans/{id}/purchases:
     parameters:
@@ -449,11 +457,12 @@ paths:
             The plan is disabled, has unsupported scheduling settings, no target
             accounts or insufficient remaining steps, or the requested steps are
             occupied. Automatic plans also require a valid commitment cap and
-            the global automatic purchasing switch.
+            the global automatic purchasing switch. A plan managed by the
+            schedule editor is refused whole (code editor_reconciliation_required).
           content:
             application/json:
               schema:
-                $ref: '#/components/schemas/Error'
+                $ref: '#/components/schemas/ErrorWithCode'
 
   # ---- Purchase Actions ---------------------------------------------------
   /api/purchases/execute:
@@ -911,6 +920,8 @@ paths:
           $ref: '#/components/responses/Forbidden'
         '404':
           $ref: '#/components/responses/NotFound'
+        '409':
+          $ref: '#/components/responses/EditorReconciliationRequired'
 
   /api/purchases/planned/{id}/run:
     parameters:
@@ -2348,6 +2359,14 @@ components:
         application/json:
           schema:
             $ref: '#/components/schemas/Error'
+    EditorReconciliationRequired:
+      description: >
+        The plan is managed by the schedule editor and this legacy change
+        needs the editor (code editor_reconciliation_required).
+      content:
+        application/json:
+          schema:
+            $ref: '#/components/schemas/ErrorWithCode'
     InternalError:
       description: Internal server error
       content:
@@ -2425,6 +2444,16 @@ components:
         error:
           type: string
 
+    ErrorWithCode:
+      type: object
+      required: [error]
+      properties:
+        error:
+          type: string
+        code:
+          type: string
+          description: Machine-readable reason, for example editor_reconciliation_required.
+
     StatusResponse:
       type: object
       required: [status]