fix(plans): eliminate universal plans — require target_accounts on creation

frontend/src/__tests__/api.test.ts

@@ -767,6 +767,7 @@ describe('Plans API', () => {
         notification_days_before: 3,
         services: { 'aws:ec2': { provider: 'aws', service: 'ec2', enabled: true, term: 3, payment: 'all-upfront', coverage: 80 } },
         ramp_schedule: { type: 'immediate', percent_per_step: 100, step_interval_days: 0, current_step: 0, total_steps: 1 },
+        target_accounts: ['11111111-1111-1111-1111-111111111111'],
       };
       await createPlan(plan);
 
@@ -789,6 +790,7 @@ describe('Plans API', () => {
         notification_days_before: 5,
         services: { 'aws:rds': { provider: 'aws', service: 'rds', enabled: true, term: 1, payment: 'no-upfront', coverage: 70 } },
         ramp_schedule: { type: 'weekly', percent_per_step: 25, step_interval_days: 7, current_step: 0, total_steps: 4 },
+        target_accounts: ['22222222-2222-2222-2222-222222222222'],
       };
       await updatePlan('plan-123', plan);
 

frontend/src/__tests__/plans.test.ts

@@ -148,6 +148,13 @@ describe('Plans Module', () => {
             <input type="number" id="ramp-step-percent" value="20">
             <input type="number" id="ramp-interval-days" value="7">
           </div>
+          <!-- Target Accounts section (universal-plans fix). The hidden
+               plan-account-ids field is the contract between renderPlan
+               AccountChips and savePlan; the submit button's disabled
+               state is recomputed every time the chip list changes. -->
+          <div id="plan-accounts-selected" class="selected-accounts"></div>
+          <input type="hidden" id="plan-account-ids" value="">
+          <button type="submit">Save Plan</button>
         </form>
       </div>
       <div id="purchase-modal" class="hidden"></div>
@@ -885,6 +892,11 @@ describe('Plans Module', () => {
       (document.getElementById('plan-auto-purchase') as HTMLInputElement).checked = true;
       (document.getElementById('plan-notify-days') as HTMLInputElement).value = '3';
       (document.getElementById('plan-enabled') as HTMLInputElement).checked = true;
+      // Universal-plans fix: savePlan rejects an empty Target Accounts list,
+      // so default the hidden field to a single account UUID for every test
+      // in this block. Tests that exercise the empty-accounts rejection set
+      // it back to '' explicitly inside the test.
+      (document.getElementById('plan-account-ids') as HTMLInputElement).value = '11111111-1111-1111-1111-111111111111';
     });
 
     test('prevents default form submission', async () => {
@@ -943,6 +955,15 @@ describe('Plans Module', () => {
       }));
     });
 
+    // Helper: openCreatePlanModal/openNewPlanModal call form.reset(), which
+    // clears the hidden plan-account-ids field stamped by the beforeEach.
+    // Universal-plans fix requires that field to be non-empty at savePlan
+    // time, so any test that opens the modal must re-stamp it before submit.
+    const stampAccountIds = () => {
+      (document.getElementById('plan-account-ids') as HTMLInputElement).value
+        = '11111111-1111-1111-1111-111111111111';
+    };
+
     test('includes the snapshot stamped by openCreatePlanModal (#273 CR)', async () => {
       // #273 CR follow-up: savePlan now reads the snapshot stamped at
       // Plan-button click time via openCreatePlanModal(snapshot), instead
@@ -956,6 +977,7 @@ describe('Plans Module', () => {
         { id: 'rec-2', service: 'rds' },
       ] as unknown as readonly api.Recommendation[];
       openCreatePlanModal(snapshot);
+      stampAccountIds();
 
       (api.createPlan as jest.Mock).mockResolvedValue({});
       (api.getPlans as jest.Mock).mockResolvedValue({ plans: [] });
@@ -980,6 +1002,7 @@ describe('Plans Module', () => {
         { id: 'rec-2', service: 'rds' },
       ] as unknown as readonly api.Recommendation[];
       openCreatePlanModal(snapshotAtClickTime);
+      stampAccountIds();
 
       // Now simulate post-modal-open state mutations: deselection,
       // refresh-replaced visible set, etc. None of these should affect
@@ -1015,6 +1038,7 @@ describe('Plans Module', () => {
       // (the New-Plan-from-scratch path explicitly clears the cache so a
       // subsequent New-Plan submit doesn't inherit a previous flow's recs).
       openNewPlanModal();
+      stampAccountIds();
       (api.createPlan as jest.Mock).mockResolvedValue({});
       (api.getPlans as jest.Mock).mockResolvedValue({ plans: [] });
       (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
@@ -1032,6 +1056,7 @@ describe('Plans Module', () => {
       // time was empty for some reason), savePlan must still submit without
       // a recommendations field, not blow up.
       openCreatePlanModal([] as unknown as readonly api.Recommendation[]);
+      stampAccountIds();
       (api.createPlan as jest.Mock).mockResolvedValue({});
       (api.getPlans as jest.Mock).mockResolvedValue({ plans: [] });
       (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
@@ -1130,6 +1155,49 @@ describe('Plans Module', () => {
       expect(api.updatePlan).toHaveBeenCalledWith('plan-123', expect.any(Object));
       expect(mockOpenArcheraOfferModal).not.toHaveBeenCalled();
     });
+
+    test('rejects submit and never calls createPlan when Target Accounts is empty (universal-plans fix)', async () => {
+      // Universal plans (purchase_plans rows with no plan_accounts row) are
+      // no longer allowed. The Save Plan button is also disabled in this
+      // state via refreshPlanSaveButtonState; this assertion is the defence-
+      // in-depth at the savePlan layer for scripted submissions or any
+      // future regression that bypasses the disabled UI.
+      (document.getElementById('plan-account-ids') as HTMLInputElement).value = '';
+      (api.createPlan as jest.Mock).mockResolvedValue({ id: 'p1' });
+
+      const event = { preventDefault: jest.fn() } as unknown as Event;
+      await savePlan(event);
+
+      expect(api.createPlan).not.toHaveBeenCalled();
+      expect(api.setPlanAccounts).not.toHaveBeenCalled();
+      expect(mockShowToast).toHaveBeenCalledWith(expect.objectContaining({
+        kind: 'error',
+        message: expect.stringContaining('Target Accounts'),
+      }));
+    });
+
+    test('forwards selected target_accounts on createPlan (universal-plans fix)', async () => {
+      // Verifies the new wire contract: savePlan stamps the selected account
+      // chip IDs onto the request body so the backend can validate and
+      // persist plan_accounts in the same call. The 2-step PUT remains a
+      // belt-and-suspenders write for update flows, but the create path
+      // must include target_accounts inline.
+      (document.getElementById('plan-account-ids') as HTMLInputElement).value
+        = '11111111-1111-1111-1111-111111111111,22222222-2222-2222-2222-222222222222';
+      (api.createPlan as jest.Mock).mockResolvedValue({ id: 'p1' });
+      (api.getPlans as jest.Mock).mockResolvedValue({ plans: [] });
+      (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
+
+      const event = { preventDefault: jest.fn() } as unknown as Event;
+      await savePlan(event);
+
+      expect(api.createPlan).toHaveBeenCalledWith(expect.objectContaining({
+        target_accounts: [
+          '11111111-1111-1111-1111-111111111111',
+          '22222222-2222-2222-2222-222222222222',
+        ],
+      }));
+    });
   });
 
   describe('closePlanModal', () => {

frontend/src/api/types.ts

@@ -168,6 +168,11 @@ export interface CreatePlanRequest {
   notification_days_before: number;
   services: Record<string, ServiceConfig>;
   ramp_schedule: PlanRampSchedule;
+  // Required server-side (universal-plans fix): a plan must be tied to at
+  // least one cloud account. The frontend bundles the selected account IDs
+  // here so the backend receives plan creation + account assignment in a
+  // single request and can validate atomically.
+  target_accounts: string[];
 }
 
 // History types

frontend/src/index.html

@@ -869,8 +869,8 @@ <h3>Automation Settings</h3>
                     </div>
 
                     <div class="form-section">
-                        <h3>Target Accounts</h3>
-                        <p class="help-text">Leave empty to target all enabled accounts for the selected provider.</p>
+                        <h3>Target Accounts <span aria-hidden="true" class="required-asterisk">*</span></h3>
+                        <p class="help-text">Required. Select at least one account this plan will purchase for. The Save button stays disabled until you add one.</p>
                         <div class="plan-accounts-search">
                             <input type="text" id="plan-account-search" placeholder="Search accounts to add...">
                             <div id="plan-account-suggestions" class="account-suggestions hidden"></div>

frontend/src/plans.ts

@@ -625,6 +625,22 @@ export async function savePlan(e: Event): Promise<void> {
     plan.recommendations = [...pendingPlanRecommendations];
   }
 
+  // Universal-plans fix: read the selected account chips and reject submit
+  // when the list is empty. The Save button is also disabled in the same
+  // condition via refreshPlanSaveButtonState() so this branch is mostly a
+  // belt-and-suspenders against scripted form submission; the toast keeps
+  // the failure mode loud either way.
+  const accountIdsField = document.getElementById('plan-account-ids') as HTMLInputElement | null;
+  const accountIds = accountIdsField?.value ? accountIdsField.value.split(',').filter(Boolean) : [];
+  if (accountIds.length === 0) {
+    showToast({
+      message: 'Target Accounts is required: pick at least one account before saving the plan.',
+      kind: 'error',
+    });
+    return;
+  }
+  plan.target_accounts = accountIds;
+
   try {
     let savedPlanId = planId;
     if (planId) {
@@ -634,8 +650,12 @@ export async function savePlan(e: Event): Promise<void> {
       savedPlanId = created.id;
     }
 
-    const accountIdsField = document.getElementById('plan-account-ids') as HTMLInputElement | null;
-    const accountIds = accountIdsField?.value ? accountIdsField.value.split(',').filter(Boolean) : [];
+    // On update, the create path's atomic plan_accounts insert doesn't fire
+    // (we only POST /plans on create). For updates we still need to push the
+    // selected account list via the dedicated endpoint. On create, we also
+    // re-push here so that subsequent reselection is reflected even if the
+    // backend later opens an "atomic-create-only" path that diverges from
+    // PUT semantics — same call already handles dedupe via DELETE+INSERT.
     if (savedPlanId) {
       await api.setPlanAccounts(savedPlanId, accountIds);
     }
@@ -650,6 +670,21 @@ export async function savePlan(e: Event): Promise<void> {
   }
 }
 
+// refreshPlanSaveButtonState toggles the Save button's disabled state based
+// on whether at least one Target Account is selected. Universal plans (rows
+// in purchase_plans with no plan_accounts row) are no longer allowed by the
+// API; surfacing the failure in the disabled state is friendlier than
+// letting the user fill in every other field and get rejected at submit.
+function refreshPlanSaveButtonState(): void {
+  const form = document.getElementById('plan-form') as HTMLFormElement | null;
+  if (!form) return;
+  const submitBtn = form.querySelector<HTMLButtonElement>('button[type="submit"]');
+  if (!submitBtn) return;
+  const hasAccounts = planSelectedAccounts.length > 0;
+  submitBtn.disabled = !hasAccounts;
+  submitBtn.title = hasAccounts ? '' : 'Select at least one Target Account to save the plan';
+}
+
 /**
  * Close plan modal
  */
@@ -697,6 +732,9 @@ function renderPlanAccountChips(): void {
 function updatePlanAccountIdsField(): void {
   const field = document.getElementById('plan-account-ids') as HTMLInputElement | null;
   if (field) field.value = planSelectedAccounts.map(a => a.id).join(',');
+  // Recalc Save-button disabled state every time the account list changes
+  // so the user gets immediate feedback when they remove the last chip.
+  refreshPlanSaveButtonState();
 }
 
 let planAccountSearchTimer: ReturnType<typeof setTimeout> | null = null;

frontend/src/types.ts

@@ -152,6 +152,11 @@ export interface SavePlanData {
   custom_step_percent?: number;
   custom_interval_days?: number;
   recommendations?: api.Recommendation[];
+  // UUIDs of cloud accounts the plan will purchase for. The server now
+  // requires at least one (universal-plans fix). The modal renders selected
+  // accounts as chips; savePlan reads them out of the hidden #plan-account-
+  // ids field and stamps the array here before POST /plans.
+  target_accounts?: string[];
 }
 
 // History types

internal/api/handler_accounts.go

@@ -1224,6 +1224,14 @@ func (h *Handler) setPlanAccounts(ctx context.Context, httpReq *events.LambdaFun
 		return nil, NewClientError(400, "invalid request body")
 	}
 
+	// Reject empty account_ids: a plan must remain tied to at least one
+	// cloud_account row. Allowing the PUT to clear all rows would recreate
+	// the universal-plan bug class (purchase_plans row with no matching
+	// plan_accounts row) that createPlan now refuses at insert time.
+	if len(body.AccountIDs) == 0 {
+		return nil, NewClientError(400, "account_ids is required: a plan must be tied to at least one account")
+	}
+
 	for _, aid := range body.AccountIDs {
 		if err := validateUUID(aid); err != nil {
 			return nil, NewClientError(400, fmt.Sprintf("invalid account_id %q: must be a valid UUID", aid))

internal/api/handler_accounts_router_test.go

@@ -112,7 +112,12 @@ func TestRouterDispatch_PlanAccountsPUT_CorrectDispatch(t *testing.T) {
 	r := setupRouterForDispatch(ctx)
 
 	planID := "22222222-2222-2222-2222-222222222222"
-	req, method, path := routerReq("PUT", "/api/plans/"+planID+"/accounts", `{"account_ids":[]}`)
+	// Body uses a single valid UUID — universal-plans fix rejects empty
+	// account_ids as a 400, which would mask a routing-misdispatch bug we
+	// actually want this test to catch. Dispatch verification still works
+	// with any well-formed body.
+	body := `{"account_ids":["11111111-1111-1111-1111-111111111111"]}`
+	req, method, path := routerReq("PUT", "/api/plans/"+planID+"/accounts", body)
 	_, err := r.Route(ctx, method, path, req)
 	require.NoError(t, err, "plan/accounts PUT should not error — invalid ID means wrong handler")
 }

internal/api/handler_accounts_test.go

@@ -1028,6 +1028,33 @@ func TestSetPlanAccounts_EmptyServicesSkipsValidation(t *testing.T) {
 	assert.Equal(t, []string{azureAcct1}, capturedIDs, "empty services map → validation skipped → write proceeds")
 }
 
+// TestSetPlanAccounts_RejectsEmpty verifies the universal-plans fix:
+// PUT /api/plans/:id/accounts with an empty account_ids returns HTTP 400
+// and never reaches the store. Eliminates the back-door for re-creating a
+// universal plan by updating an existing one to have zero target accounts.
+func TestSetPlanAccounts_RejectsEmpty(t *testing.T) {
+	ctx := context.Background()
+	mockAuth := new(MockAuthService)
+	setupAdminAuth(ctx, mockAuth)
+
+	setCalled := false
+	store := setupAdminMock(ctx)
+	store.SetPlanAccountsFn = func(_ context.Context, _ string, _ []string) error {
+		setCalled = true
+		return nil
+	}
+	handler := &Handler{auth: mockAuth, config: store}
+
+	body := `{"account_ids":[]}`
+	_, err := handler.setPlanAccounts(ctx, adminRequest(body), planID209)
+	require.Error(t, err)
+	ce, ok := IsClientError(err)
+	require.True(t, ok)
+	assert.Equal(t, 400, ce.code)
+	assert.Contains(t, ce.Error(), "account_ids is required")
+	assert.False(t, setCalled, "SetPlanAccounts must NOT be called when account_ids is empty")
+}
+
 func TestListPlanAccounts_Success(t *testing.T) {
 	ctx := context.Background()
 	mockAuth := new(MockAuthService)

internal/api/handler_plans.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/LeanerCloud/CUDly/internal/config"
 	"github.com/LeanerCloud/CUDly/pkg/common"
+	"github.com/LeanerCloud/CUDly/pkg/logging"
 	"github.com/aws/aws-lambda-go/events"
 	"github.com/google/uuid"
 	"github.com/jackc/pgx/v5"
@@ -72,6 +73,16 @@ func (h *Handler) createPlan(ctx context.Context, httpReq *events.LambdaFunction
 		return nil, NewClientError(400, "invalid request body")
 	}
 
+	// target_accounts is required: a plan must be tied to at least one
+	// cloud_account row. The historical "leave blank to mean all accounts of
+	// this provider" behaviour created "universal plans" (rows in
+	// purchase_plans with no matching plan_accounts row) that were hard to
+	// scope, hard to filter, and hard to govern. Reject early with a clear
+	// 400 so the frontend can surface the error before any DB write.
+	if err := validateTargetAccounts(req.TargetAccounts); err != nil {
+		return nil, err
+	}
+
 	plan := req.toPurchasePlan()
 
 	// Validate the plan
@@ -83,9 +94,53 @@ func (h *Handler) createPlan(ctx context.Context, httpReq *events.LambdaFunction
 		return nil, err
 	}
 
+	// Provider-match validation + plan_accounts insert. SetPlanAccounts is
+	// transactional internally, but the plan-row insert above is not part of
+	// that tx. If either step here fails we roll the plan row back so the
+	// invariant "every purchase_plans row has at least one plan_accounts
+	// row" holds end-to-end — otherwise a validation failure would leave a
+	// fresh universal plan behind, which is exactly the bug class we're
+	// eliminating.
+	//
+	// rollbackPlan undoes the partial CreatePurchasePlan insert. If the
+	// rollback delete itself errors (DB blip, row already gone, etc.), log
+	// at WARN with the plan ID so an operator can clean up manually — we
+	// still surface the original cause to the caller so the user-facing
+	// error is unchanged.
+	rollbackPlan := func() {
+		if delErr := h.config.DeletePurchasePlan(ctx, plan.ID); delErr != nil {
+			logging.Warnf("createPlan rollback: failed to delete partial plan %s: %v (manual cleanup may be required)", plan.ID, delErr)
+		}
+	}
+	if err := h.validatePlanAccountProviders(ctx, plan.ID, req.TargetAccounts); err != nil {
+		rollbackPlan()
+		return nil, err
+	}
+	if err := h.config.SetPlanAccounts(ctx, plan.ID, req.TargetAccounts); err != nil {
+		rollbackPlan()
+		return nil, fmt.Errorf("accounts: %w", err)
+	}
+
 	return plan, nil
 }
 
+// validateTargetAccounts rejects a missing/empty target_accounts payload and
+// rejects entries that are not valid UUIDs. Mirrors the validation the
+// dedicated PUT /plans/:id/accounts endpoint already performs (see
+// setPlanAccounts in handler_accounts.go) so a request that gets past
+// createPlan would also get past that endpoint.
+func validateTargetAccounts(ids []string) error {
+	if len(ids) == 0 {
+		return NewClientError(400, "target_accounts is required: a plan must be tied to at least one account")
+	}
+	for _, aid := range ids {
+		if err := validateUUID(aid); err != nil {
+			return NewClientError(400, fmt.Sprintf("invalid target_account %q: must be a valid UUID", aid))
+		}
+	}
+	return nil
+}
+
 func (h *Handler) getPlan(ctx context.Context, req *events.LambdaFunctionURLRequest, planID string) (any, error) {
 	// Validate UUID format to prevent injection attacks
 	if err := validateUUID(planID); err != nil {