feat(commitmentopts): use AWS SP probe in Validate path (closes #134)

internal/commitmentopts/service.go

@@ -163,13 +163,14 @@ func (s *Service) Validate(ctx context.Context, provider, service string, term i
 	}
 	combos, ok := byService[service]
 	if !ok {
-		// We have probe data for this provider but not for this service.
-		// This typically means the service is not commitment-capable per
-		// the probe (e.g. Savings Plans has no per-service offering list)
-		// or it is a new service not yet covered by the probe set.
-		// Log at Warn so operators can detect misconfigured service names
-		// or probe gaps without blocking the plan save (05-M4). The
-		// frontend's hardcoded rules are the primary user-facing gate.
+		// We have data for this provider but not this service key.
+		// The probe ran but returned no combos for this service
+		// (e.g. a plan type not sold in the account's region).
+		// Permissive fallback keeps parity with ErrNoData so unknown
+		// services are never silently blocked. Log at Warn so operators
+		// can detect misconfigured service names or probe gaps without
+		// blocking the plan save (05-M4). The frontend's hardcoded
+		// rules are the primary user-facing gate.
 		logging.Warnf("commitmentopts: provider %q known in probe data but service %q absent; treating as valid (term=%d payment=%q)",
 			provider, service, term, payment)
 		return true, nil

internal/commitmentopts/service_test.go

@@ -241,12 +241,116 @@ func TestService_Validate_UnknownServiceUnderKnownProviderPermissive(t *testing.
 	store := &fakeStore{opts: cached, has: true}
 	svc := New(store, &fakeAccounts{}, noopBuildConfig, nil)
 
-	// savingsplans has no probe — don't block.
+	// A service key absent from the probe snapshot must not block saves.
 	ok, err := svc.Validate(context.Background(), "aws", "savingsplans", 1, "all-upfront")
 	require.NoError(t, err)
 	assert.True(t, ok)
 }
 
+// TestService_Validate_SavingsPlansProbeHit verifies that Validate consults
+// the probe snapshot for SP service keys and rejects combinations that the
+// probe did not return. Precedence: probe data wins over permissive fallback
+// when the key is present in the snapshot.
+func TestService_Validate_SavingsPlansProbeHit(t *testing.T) {
+	// Seed a snapshot that only has 1yr/all-upfront for savings-plans-compute.
+	cached := Options{"aws": {
+		"savings-plans-compute": {
+			{Provider: "aws", Service: "savings-plans-compute", TermYears: 1, Payment: "all-upfront"},
+			{Provider: "aws", Service: "savings-plans-compute", TermYears: 1, Payment: "no-upfront"},
+		},
+	}}
+	store := &fakeStore{opts: cached, has: true}
+	svc := New(store, &fakeAccounts{}, noopBuildConfig, nil)
+
+	// Probe returned this combo — must be valid.
+	ok, err := svc.Validate(context.Background(), "aws", "savings-plans-compute", 1, "all-upfront")
+	require.NoError(t, err)
+	assert.True(t, ok, "combo present in probe snapshot must validate")
+
+	// Probe did NOT return 3yr/partial-upfront — must be rejected.
+	ok, err = svc.Validate(context.Background(), "aws", "savings-plans-compute", 3, "partial-upfront")
+	require.NoError(t, err)
+	assert.False(t, ok, "combo absent from probe snapshot must be rejected")
+}
+
+// TestService_Validate_SavingsPlansKeyMissingInSnapshot verifies that
+// Validate falls back to permissive-true when the probe ran successfully but
+// returned no combos for a specific SP service key (e.g. a plan type not
+// offered in the account's region). The presence of another SP key in the
+// snapshot confirms the probe did run; the missing key means "no data for
+// this plan type", not "probe not run".
+func TestService_Validate_SavingsPlansKeyMissingInSnapshot(t *testing.T) {
+	// Probe ran and stored combos for savings-plans-compute only.
+	cached := Options{"aws": {
+		"savings-plans-compute": {
+			{Provider: "aws", Service: "savings-plans-compute", TermYears: 1, Payment: "all-upfront"},
+		},
+	}}
+	store := &fakeStore{opts: cached, has: true}
+	svc := New(store, &fakeAccounts{}, noopBuildConfig, nil)
+
+	// savings-plans-database is absent from the snapshot — permissive fallback.
+	ok, err := svc.Validate(context.Background(), "aws", "savings-plans-database", 1, "all-upfront")
+	require.NoError(t, err)
+	assert.True(t, ok, "absent service key must not block saves (permissive fallback)")
+}
+
+// TestService_Validate_SavingsPlansErrNoDataFallback verifies that Validate
+// returns permissive-true for SP service keys when no probe data exists at all
+// (ErrNoData path). This covers the cold-start case where the probe has never
+// run — saves must not be blocked.
+func TestService_Validate_SavingsPlansErrNoDataFallback(t *testing.T) {
+	// Cold store: no probe run, no AWS account to probe.
+	store := &fakeStore{}
+	accounts := &fakeAccounts{accounts: nil}
+	svc := New(store, accounts, noopBuildConfig, nil)
+
+	for _, spService := range []string{
+		"savings-plans-compute",
+		"savings-plans-ec2instance",
+		"savings-plans-sagemaker",
+		"savings-plans-database",
+	} {
+		ok, err := svc.Validate(context.Background(), "aws", spService, 1, "all-upfront")
+		require.NoError(t, err, "service=%s", spService)
+		assert.True(t, ok, "ErrNoData must not block saves for %s", spService)
+	}
+}
+
+// TestService_Validate_SavingsPlansProbeConsulted verifies the probe-first
+// precedence for SP keys end-to-end: cold store + live SP prober mock
+// -> probe fires -> Validate uses the probe result, not the permissive default.
+// Precedence chain: probe data in snapshot -> permissive fallback (ErrNoData
+// or absent key).
+func TestService_Validate_SavingsPlansProbeConsulted(t *testing.T) {
+	store := &fakeStore{}
+	accounts := &fakeAccounts{accounts: []config.CloudAccount{awsAccount("123456789012")}}
+
+	// SP prober returns only 1yr/no-upfront for savings-plans-compute.
+	spCombos := []Combo{
+		{Provider: "aws", Service: "savings-plans-compute", TermYears: 1, Payment: "no-upfront"},
+	}
+	probers := []Prober{
+		&stubProber{name: "savings-plans", combos: spCombos},
+	}
+	svc := New(store, accounts, noopBuildConfig, probers)
+
+	// Trigger probe by calling Get (cold store).
+	opts, err := svc.Get(context.Background())
+	require.NoError(t, err)
+	require.Contains(t, opts["aws"], "savings-plans-compute",
+		"probe result must land under its per-product service key")
+
+	// The probe snapshot now governs Validate.
+	ok, err := svc.Validate(context.Background(), "aws", "savings-plans-compute", 1, "no-upfront")
+	require.NoError(t, err)
+	assert.True(t, ok, "1yr/no-upfront was returned by probe — must validate")
+
+	ok, err = svc.Validate(context.Background(), "aws", "savings-plans-compute", 3, "all-upfront")
+	require.NoError(t, err)
+	assert.False(t, ok, "3yr/all-upfront was NOT returned by probe — must be rejected")
+}
+
 func TestService_Get_ConcurrentCallersProbeOnce(t *testing.T) {
 	// Serialize probes via the mutex: N concurrent Get()s on a cold store
 	// must result in exactly one Save.