feat(purchase): apply the step cap to bound plans per group

internal/api/handler_purchases.go

@@ -1504,7 +1504,7 @@ func (h *Handler) requirePlanStepCap(ctx context.Context, execution *config.Purc
 	if err != nil {
 		return fmt.Errorf("failed to read plan %s for the approval cap check: %w", execution.PlanID, err)
 	}
-	if !plan.AutoPurchase && plan.MaxStepCommitment == nil {
+	if !purchase.StepCapApplies(plan) {
 		return nil
 	}
 	if capErr := purchase.CheckStepCommitmentCap(plan, execution.Recommendations); capErr != nil {

internal/api/handler_purchases_plan_step_approve_test.go

@@ -279,3 +279,33 @@ func TestApproveConstraintsForSession_NilSession_NonUSDCappedPlanStep(t *testing
 	_, err := newTokenApprove(t, nil, autoPlan(&capAmount)).handler.approveViaToken(ctx, &events.LambdaFunctionURLRequest{}, exec, "valid-token")
 	requireStatus(t, err, 404) // the 401 is concealed as not-found
 }
+
+// A manual bound plan with group caps and no plan cap: the approval gate runs
+// only because it shares StepCapApplies with the attach path (#1746), so an
+// over-cap group, or a rec outside every group, is refused for the email-link
+// approver too.
+func TestApproveViaToken_PlanStep_ManualBoundPlanGroupCaps(t *testing.T) {
+	exec, total := planStepExecution(t)
+	key, err := config.RecIdentityKey(&exec.Recommendations[0])
+	require.NoError(t, err)
+	bound := func(groupCap *float64, ids ...string) *config.PurchasePlan {
+		return &config.PurchasePlan{
+			ID:           stepPlanID,
+			RampSchedule: config.RampSchedule{AllocationMode: config.RampAllocationBoundGroups},
+			Services: map[string]config.ServiceConfig{"azure/compute#g1": {
+				Provider: "azure", Service: "compute", Enabled: true, MinCount: 1, GroupID: "g1",
+				BoundIdentities: ids, MaxStepCommitment: groupCap,
+			}},
+		}
+	}
+	under, over := total+1, total-1
+
+	f := newTokenApprove(t, nil, bound(&under, key))
+	requireApproveReached(t, f.purchase, f.approve(exec))
+
+	f = newTokenApprove(t, nil, bound(&over, key))
+	requireApproveRefused(t, f.purchase, f.approve(exec), 403)
+
+	f = newTokenApprove(t, nil, bound(&under, "some-other-identity"))
+	requireApproveRefused(t, f.purchase, f.approve(exec), 403)
+}

internal/purchase/errnotes.go

@@ -25,7 +25,7 @@ var serverNotePatterns = []string{
 	regexp.QuoteMeta(armedRedriveLookupRefusal),
 	`stale Azure recommendation pricing; refresh recommendations and submit a new purchase(?:: the latest collection state is missing, so Azure prices cannot be confirmed current)?`,
 	`(?:execution ` + uuidPattern + `: )?plan step has no recommendations attached, so it cannot buy anything(?: \(platform#631: plan steps cannot yet carry recommendations\))?`,
-	`plan step refused by commitment cap(?:: plan has no max_step_commitment set \(auto-purchase of steps with recommendations is refused until one is set\)|: step total commitment \d+\.\d{2} exceeds max_step_commitment \d+\.\d{2})?`,
+	`plan step refused by commitment cap(?:: plan has no max_step_commitment set \(auto-purchase of steps with recommendations is refused until one is set\)|: step total commitment \d+\.\d{2} exceeds max_step_commitment \d+\.\d{2}|` + boundGroupCapNotePatterns + `)?`,
 	`scheduled purchase skipped: it was due too long ago(?:: it was due at \d{4}-\d{2}-\d{2}T[0-9:.]+Z, [0-9a-z.]+ ago, which is more than the [0-9a-z.]+ limit \(SCHEDULED_PURCHASE_MAX_OVERDUE\); nothing was submitted, retry it if the purchase is still wanted)?`,
 	`purchase refused after claim: the claimed execution no longer passes the authorization checks`,
 	`refusing to execute ladder purchase: (?:global config unavailable|ladder planning is switched off|ladder execution is switched off|its tranche status is unknown|its tranche is [a-z_]+|its ladder config no longer exists|its ladder config is disabled|ladder config mode is "[a-z_]*", not auto_approve|its ladder run no longer exists|its ladder run was canceled|its ladder run status "[a-z_]{1,32}" is unknown)`,
@@ -101,6 +101,16 @@ const (
 	// whitespace, quotes or newline. Anything wider falls back to the fixed head.
 	amountPattern     = `\d+(?:\.\d+)?`
 	shortValuePattern = `[A-Za-z0-9._+-]{1,32}`
+	// groupKeyPattern is a bound group's services key ("aws/ec2#g1"); a key with
+	// any other character falls back to the fixed head.
+	groupKeyPattern = `[A-Za-z0-9._:/#+-]{1,128}`
+	// boundGroupCapNotePatterns are the group-level refusals of a bound plan's step
+	// cap (step_cap.go): a group over or without a usable cap, or a rec in two
+	// groups. They name only group keys and numbers. "Outside every group" names a
+	// rec id, which carries an account, so it keeps only the head.
+	boundGroupCapNotePatterns = `: group "` + groupKeyPattern + `" step commitment \d+\.\d{2} exceeds its max_step_commitment \d+\.\d{2}` +
+		`|: group "` + groupKeyPattern + `" has no usable max_step_commitment \((?:NaN|[+-]?Inf|-?\d+(?:\.\d+)?(?:e[+-]\d+)?)\)` +
+		`|: a recommendation is bound by two groups \("` + groupKeyPattern + `" and "` + groupKeyPattern + `"\)`
 	// uuidPattern is a Postgres UUID as it renders (plan, execution and cloud
 	// account IDs). commitmentIDPattern covers provider commitment IDs and ARNs;
 	// identityKeyPattern a RecIdentityKey with "/" separators (skippedNote).

internal/purchase/plan_step_attach.go

@@ -184,7 +184,7 @@ func (m *Manager) attachStep(ctx context.Context, tx pgx.Tx, locked *config.Purc
 }
 
 func (m *Manager) attachWithinCap(ctx context.Context, tx pgx.Tx, locked *config.PurchasePlan, claim stepClaim, root *config.PurchaseExecution, recs []config.RecommendationRecord) (stepClaim, error) {
-	if locked.AutoPurchase || locked.MaxStepCommitment != nil {
+	if StepCapApplies(locked) {
 		if capErr := CheckStepCommitmentCap(locked, recs); capErr != nil {
 			return m.failStep(ctx, tx, locked, claim, root, fmt.Sprintf("%v (recommendations: %s)", capErr, recIDs(recs)))
 		}

internal/purchase/plan_step_attach_integration_test.go

@@ -152,6 +152,40 @@ func TestAttachIntegration_NoMatchAndCap(t *testing.T) {
 		assert.Contains(t, rows[0].Error, "rec-a")
 		assert.Contains(t, rows[0].Error, "exceeds max_step_commitment 1.00")
 	})
+	t.Run("a manual bound plan with an over-cap group and no plan cap fails the step", func(t *testing.T) {
+		// Only StepCapApplies makes the attach path run the cap check for this plan
+		// (manual, no plan cap): its group cap and its group mapping are held here too.
+		rec := attachRec("rec-a", 5)
+		key, err := config.RecIdentityKey(&rec)
+		require.NoError(t, err)
+		next := time.Now().Add(24 * time.Hour).Truncate(time.Microsecond)
+		plan := &config.PurchasePlan{
+			Name: "attach-bound-manual", Enabled: true, NotificationDaysBefore: 30,
+			Services: map[string]config.ServiceConfig{"aws/ec2#g1": {
+				Provider: "aws", Service: "ec2", Enabled: true, Term: 1, Payment: "no-upfront", Coverage: 80,
+				MinCount: 1, GroupID: "g1", BoundIdentities: []string{key}, MaxStepCommitment: f64(1),
+			}},
+			RampSchedule:      config.RampSchedule{Type: "weekly", PercentPerStep: 25, StepIntervalDays: 7, TotalSteps: 4, StartDate: next.AddDate(0, 0, -7), AllocationMode: config.RampAllocationBoundGroups},
+			NextExecutionDate: &next,
+		}
+		require.NoError(t, plan.Validate())
+		require.NoError(t, store.CreatePurchasePlan(ctx, plan))
+		// A manual plan attaches only into its pre-created empty root row.
+		root := &config.PurchaseExecution{PlanID: plan.ID, ExecutionID: "00000000-0000-4000-8000-0000000000a2", Status: "pending", StepNumber: 1, ScheduledDate: time.Now().Add(40 * 24 * time.Hour)}
+		require.NoError(t, store.SavePurchaseExecution(ctx, root))
+		sender := newCountingSender(ctx)
+		m := &Manager{config: store, email: sender, resolver: &fakeResolver{recs: []config.RecommendationRecord{rec}}}
+
+		require.False(t, m.sendPlanNotification(ctx, plan, false))
+
+		rows := stepRowsInDB(ctx, t, store, plan.ID)
+		require.Len(t, rows, 1)
+		assert.Equal(t, root.ExecutionID, rows[0].ExecutionID)
+		assert.Equal(t, "failed", rows[0].Status)
+		assert.Empty(t, rows[0].Recommendations)
+		assert.Contains(t, rows[0].Error, `group "aws/ec2#g1"`)
+		assert.Contains(t, rows[0].Error, "exceeds its max_step_commitment 1.00")
+	})
 	t.Run("pre-created empty root row is attached in place", func(t *testing.T) {
 		plan := attachPlanInDB(ctx, t, store, "attach-root", time.Now().Add(24*time.Hour))
 		root := &config.PurchaseExecution{PlanID: plan.ID, ExecutionID: "00000000-0000-4000-8000-0000000000a1", Status: "pending", StepNumber: 1, ScheduledDate: time.Now().Add(40 * 24 * time.Hour)}

internal/purchase/step_cap.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"math"
+	"sort"
 	"strings"
 
 	"github.com/LeanerCloud/cloud-commitments-go/pkg/logging"
@@ -14,6 +15,23 @@ import (
 // ErrStepCommitmentCap marks a plan step refused by the per-plan spend cap.
 var ErrStepCommitmentCap = errors.New("plan step refused by commitment cap")
 
+// ErrRecOutsideBoundGroups marks a bound plan step (issue #1746) holding a
+// recommendation that no group bound. It wraps ErrStepCommitmentCap, so every
+// cap site refuses it the same way.
+var ErrRecOutsideBoundGroups = fmt.Errorf("%w: recommendation is outside every bound group", ErrStepCommitmentCap)
+
+// StepCapApplies reports whether CheckStepCommitmentCap must run for plan:
+// auto-purchase plans, plans with a cap, and bound plans, whose recommendations
+// must map to a group whether or not any cap is set. The attach and email
+// approval sites share it so they cannot drift.
+func StepCapApplies(plan *config.PurchasePlan) bool {
+	return plan != nil && (plan.AutoPurchase || plan.MaxStepCommitment != nil || isBoundPlan(plan))
+}
+
+func isBoundPlan(plan *config.PurchasePlan) bool {
+	return plan.RampSchedule.AllocationMode == config.RampAllocationBoundGroups
+}
+
 // CheckStepCommitmentCap refuses a plan step's recommendations unless the plan
 // carries a cap and the step's TOTAL commitment (upfront + monthly * term * 12,
 // see RecommendationRecord.TotalCommitment) fits under it. It fails closed:
@@ -32,8 +50,11 @@ func CheckStepCommitmentCap(plan *config.PurchasePlan, recs []config.Recommendat
 	if len(recs) == 0 {
 		return nil
 	}
-	if plan == nil || plan.MaxStepCommitment == nil || !(*plan.MaxStepCommitment > 0) || math.IsInf(*plan.MaxStepCommitment, 0) {
-		return fmt.Errorf("%w: plan has no max_step_commitment set (auto-purchase of steps with recommendations is refused until one is set)", ErrStepCommitmentCap)
+	if plan != nil && isBoundPlan(plan) {
+		return checkBoundStepCap(plan, recs)
+	}
+	if !planCapUsable(plan) {
+		return errNoPlanCap()
 	}
 	var total float64
 	for i := range recs {
@@ -49,6 +70,121 @@ func CheckStepCommitmentCap(plan *config.PurchasePlan, recs []config.Recommendat
 	return nil
 }
 
+func planCapUsable(plan *config.PurchasePlan) bool {
+	return plan != nil && plan.MaxStepCommitment != nil && capUsable(*plan.MaxStepCommitment)
+}
+
+// capUsable reports whether a cap is a positive finite number. NaN is not: it
+// fails every comparison, so as a cap it would silently let any step through.
+func capUsable(c float64) bool {
+	return c > 0 && !math.IsInf(c, 0)
+}
+
+func errNoPlanCap() error {
+	return fmt.Errorf("%w: plan has no max_step_commitment set (auto-purchase of steps with recommendations is refused until one is set)", ErrStepCommitmentCap)
+}
+
+// checkBoundStepCap is CheckStepCommitmentCap for a bound plan (issue #1746).
+// Every rec must belong to a group by its identity. An auto-purchase plan
+// still needs a plan cap; a manual one (a human approves each row) may have
+// none. A rec's commitment is checked only where a cap applies to it: the
+// plan cap, or its group's cap. Each group stays under its own cap, and the
+// total under the plan cap, so the effective cap is min(group, plan).
+func checkBoundStepCap(plan *config.PurchasePlan, recs []config.RecommendationRecord) error {
+	planCap := planCapUsable(plan)
+	if !planCap && (plan.AutoPurchase || plan.MaxStepCommitment != nil) {
+		return errNoPlanCap()
+	}
+	groupOf, err := boundGroupOf(plan)
+	if err != nil {
+		return err
+	}
+	totals, total, err := boundStepTotals(plan, recs, groupOf, planCap)
+	if err != nil {
+		return err
+	}
+	if planCap && total > *plan.MaxStepCommitment {
+		return fmt.Errorf("%w: step total commitment %.2f exceeds max_step_commitment %.2f", ErrStepCommitmentCap, total, *plan.MaxStepCommitment)
+	}
+	return checkBoundGroupCaps(plan, totals)
+}
+
+// boundGroupOf maps each bound identity key to the service key of its group.
+// A disabled group is skipped, as the resolver skips it, so its identities are
+// outside every group. An identity bound by two groups is refused: validation
+// rejects it, but a stored plan may hold one. Keys are visited in order so the
+// pair named in the refusal is the same on every check.
+func boundGroupOf(plan *config.PurchasePlan) (map[string]string, error) {
+	keys := make([]string, 0, len(plan.Services))
+	for k := range plan.Services {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
+	groupOf := make(map[string]string)
+	for _, k := range keys {
+		if !plan.Services[k].Enabled {
+			continue
+		}
+		for _, id := range plan.Services[k].BoundIdentities {
+			if prev, dup := groupOf[id]; dup && prev != k {
+				return nil, fmt.Errorf("%w: a recommendation is bound by two groups (%q and %q)", ErrStepCommitmentCap, prev, k)
+			}
+			groupOf[id] = k
+		}
+	}
+	return groupOf, nil
+}
+
+// boundStepTotals maps every rec to its bound group and sums the commitments a
+// cap applies to: all of them under a plan cap, otherwise only those whose
+// group has its own cap. A rec outside every group is refused.
+func boundStepTotals(plan *config.PurchasePlan, recs []config.RecommendationRecord, groupOf map[string]string, planCap bool) (totals map[string]float64, total float64, err error) {
+	totals = map[string]float64{}
+	for i := range recs {
+		key, keyErr := config.RecIdentityKey(&recs[i])
+		if keyErr != nil {
+			return nil, 0, fmt.Errorf("%w: recommendation %q: %w", ErrStepCommitmentCap, recs[i].ID, keyErr)
+		}
+		g, ok := groupOf[key]
+		if !ok {
+			return nil, 0, fmt.Errorf("%w (recommendation %q)", ErrRecOutsideBoundGroups, recs[i].ID)
+		}
+		if !planCap && plan.Services[g].MaxStepCommitment == nil {
+			continue
+		}
+		c, capErr := capCommitment(&recs[i])
+		if capErr != nil {
+			return nil, 0, capErr
+		}
+		totals[g] += c
+		total += c
+	}
+	return totals, total, nil
+}
+
+// checkBoundGroupCaps refuses the first group, in key order, whose cap is
+// unusable or that is over its own cap.
+func checkBoundGroupCaps(plan *config.PurchasePlan, totals map[string]float64) error {
+	keys := make([]string, 0, len(totals))
+	for g := range totals {
+		keys = append(keys, g)
+	}
+	sort.Strings(keys)
+	for _, g := range keys {
+		gc := plan.Services[g].MaxStepCommitment
+		if gc == nil {
+			continue
+		}
+		if !capUsable(*gc) {
+			return fmt.Errorf("%w: group %q has no usable max_step_commitment (%v)", ErrStepCommitmentCap, g, *gc)
+		}
+		if totals[g] > *gc {
+			return fmt.Errorf("%w: group %q step commitment %.2f exceeds its max_step_commitment %.2f", ErrStepCommitmentCap, g, totals[g], *gc)
+		}
+	}
+	return nil
+}
+
 // capCommitment returns a rec's total commitment for the cap comparison, or
 // refuses it. The cap is USD with no FX conversion, so a set non-USD currency
 // is refused; an empty (unknown) currency passes.