chore(purchase): drop maskActor's dead guard and inert commitmentopts helpers

internal/commitmentopts/probe.go

@@ -557,42 +557,10 @@ func (p *SavingsPlansProber) probeOnePlanType(
 	if err != nil {
 		return nil, err
 	}
-	raw = dedupeRaw(raw)
-	// Use collect but override the service key to the per-product slug.
-	combos := collectWithService(serviceKey, raw)
-	return combos, nil
-}
-
-// dedupeRaw removes duplicate (durationSeconds, payment) pairs so that
-// collect does not see the same pair multiple times across an SP type's
-// pages. SP offerings differ by ProductType (EC2, Lambda, Fargate ...) but
-// share the same (duration, payment) matrix — we only need unique combos.
-func dedupeRaw(raw []rawOffer) []rawOffer {
-	type key struct {
-		dur     int64
-		payment string
-	}
-	seen := make(map[key]struct{}, len(raw))
-	out := make([]rawOffer, 0, len(raw))
-	for _, r := range raw {
-		k := key{r.durationSeconds, r.payment}
-		if _, dup := seen[k]; dup {
-			continue
-		}
-		seen[k] = struct{}{}
-		out = append(out, r)
-	}
-	return out
-}
-
-// collectWithService is like collect but uses the supplied serviceKey
-// instead of the service embedded in each rawOffer (rawOffer has no
-// service field; collect() uses the `service` argument, which is the
-// per-prober name). Extracted to avoid a second copy of the
-// normalization loop when the service name comes from outside the
-// prober's Service() method (as it does for SP plan types).
-func collectWithService(service string, raw []rawOffer) []Combo {
-	return collect(service, raw)
+	// collect dedupes on the normalized (term, payment) pair, which also
+	// collapses the same matrix repeated across an SP type's ProductTypes
+	// (EC2, Lambda, Fargate ...) and pages.
+	return collect(serviceKey, raw), nil
 }
 
 func (p *SavingsPlansProber) client(cfg aws.Config) SavingsPlansDescribeOfferings {

internal/commitmentopts/probe_test.go

@@ -651,6 +651,69 @@ func TestSavingsPlansProber_DedupedAcrossProductTypes(t *testing.T) {
 	assert.Len(t, compute, 1, "duplicate (dur, pay) must collapse to one Combo")
 }
 
+// TestSavingsPlansProber_DuplicatesAndNormalizedCollisionsPinned pins the
+// exact, ordered output for one plan type whose pages repeat raw
+// (durationSeconds, payment) pairs and also carry different raw spellings
+// that normalize to the same (term, payment). collect() alone is the dedupe:
+// the output keeps the first occurrence of each normalized pair, in page
+// order, and drops unknown durations and legacy utilization tokens. Issue
+// #325 removed a raw-pair pre-dedupe that ran before collect(); this test
+// shows the output is unchanged without it.
+func TestSavingsPlansProber_DuplicatesAndNormalizedCollisionsPinned(t *testing.T) {
+	const twoYearSeconds = 2 * 31536000
+	sp := func(dur int64, pay string) savingsplanstypes.SavingsPlanOffering {
+		return spOffering(dur, savingsplanstypes.SavingsPlanPaymentOption(pay))
+	}
+	pages := [][]savingsplanstypes.SavingsPlanOffering{
+		{
+			sp(threeYearSeconds, "No Upfront"),
+			sp(oneYearSeconds, "All Upfront"),
+			sp(oneYearSeconds, "All Upfront"), // exact raw duplicate
+			sp(oneYearSeconds, "ALL_UPFRONT"), // normalizes to (1, all-upfront)
+			sp(twoYearSeconds, "All Upfront"), // unknown term: dropped
+			sp(twoYearSeconds, "All Upfront"), // duplicate of a dropped pair
+			sp(oneYearSeconds, "Heavy Utilization"),
+			sp(oneYearSeconds, "Heavy Utilization"),
+		},
+		{
+			sp(threeYearSeconds, "no-upfront"),      // normalized collision across pages
+			sp(oneYearSeconds, "All Upfront"),       // exact raw duplicate across pages
+			sp(oneYearSeconds, "Partial Upfront"),   // first new pair on page 2
+			sp(threeYearSeconds, "NO_UPFRONT"),      // another normalized collision
+			sp(oneYearSeconds, "partial_upfront"),   // collides with the pair above
+			sp(threeYearSeconds, "Partial Upfront"), // second new pair on page 2
+		},
+	}
+	calls := 0
+	fake := &fakeSavingsPlans{
+		fn: func(in *savingsplans.DescribeSavingsPlansOfferingsInput) (*savingsplans.DescribeSavingsPlansOfferingsOutput, error) {
+			if in.PlanTypes[0] != savingsplanstypes.SavingsPlanTypeCompute {
+				return &savingsplans.DescribeSavingsPlansOfferingsOutput{}, nil
+			}
+			out := &savingsplans.DescribeSavingsPlansOfferingsOutput{SearchResults: pages[calls]}
+			calls++
+			if calls < len(pages) {
+				out.NextToken = aws.String("page-2")
+			}
+			return out, nil
+		},
+	}
+	p := &SavingsPlansProber{NewClient: func(_ aws.Config) SavingsPlansDescribeOfferings { return fake }}
+
+	got, err := p.Probe(context.Background(), aws.Config{})
+	require.NoError(t, err)
+	require.Equal(t, len(pages), calls, "both Compute pages must be read")
+
+	const svc = "savings-plans-compute"
+	want := []Combo{
+		{Provider: "aws", Service: svc, TermYears: 3, Payment: "no-upfront"},
+		{Provider: "aws", Service: svc, TermYears: 1, Payment: "all-upfront"},
+		{Provider: "aws", Service: svc, TermYears: 1, Payment: "partial-upfront"},
+		{Provider: "aws", Service: svc, TermYears: 3, Payment: "partial-upfront"},
+	}
+	assert.Equal(t, want, got, "first occurrence of each normalized pair, in page order")
+}
+
 // TestDefaultProbers_IncludesSavingsPlans verifies that SavingsPlansProber is
 // registered in DefaultProbers.
 func TestDefaultProbers_IncludesSavingsPlans(t *testing.T) {

internal/purchase/approvals.go

@@ -98,12 +98,8 @@ func maskActor(actor string) string {
 	if actor == "" {
 		return "<anon>"
 	}
-	if at := len(actor) - 1; at >= 0 {
-		for i, c := range actor {
-			if c == '@' {
-				return "***" + actor[i:]
-			}
-		}
+	if at := strings.Index(actor, "@"); at >= 0 {
+		return "***" + actor[at:]
 	}
 	// Not an email address -- log only the last 4 chars (e.g. a username).
 	if len(actor) > 4 {

internal/purchase/approvals_test.go

@@ -1375,3 +1375,26 @@ func TestManager_ApproveAndExecute_FourEyesOn_SystemCreated(t *testing.T) {
 	require.ErrorIs(t, gate(nil, "  ", nil), ErrFourEyesDenied, "anonymous actor")
 	require.ErrorIs(t, gate(&creatorID, "", &creatorID), ErrFourEyesDenied, "flag must not override a creator")
 }
+
+// TestMaskActor pins the PII mask written to the approval logs: everything
+// before the first '@' is hidden, a non-email keeps only its last 4 bytes,
+// and an empty actor is "<anon>".
+func TestMaskActor(t *testing.T) {
+	cases := []struct{ in, want string }{
+		{"", "<anon>"},
+		{"approver@example.com", "***@example.com"},
+		{"a@b", "***@b"},
+		{"@example.com", "***@example.com"},
+		{"trailing@", "***@"},
+		{"first@second@example.com", "***@second@example.com"},
+		{"josé@example.com", "***@example.com"},
+		{"\xff\xfe@example.com", "***@example.com"},
+		{"a", "****"},
+		{"abcd", "****"},
+		{"abcde", "...bcde"},
+		{"service-account", "...ount"},
+	}
+	for _, tc := range cases {
+		assert.Equal(t, tc.want, maskActor(tc.in), "maskActor(%q)", tc.in)
+	}
+}