From 47d9bdd593920f7abb2afc8393e12fadb3a3959e Mon Sep 17 00:00:00 2001 From: Christopher Brady Date: Wed, 2 Sep 2026 14:43:29 -0600 Subject: [PATCH 1/4] deny once the overage cap is spent Overage previously failed open with no floor: enabled meant the balance stopped gating the check entirely. The cap moves the floor to -cap rather than removing it, so a company consumes past zero until it has accrued the configured limit and is then denied, the way an exhausted balance denies with overage off. An absent cap stays uncapped, which is the behaviour that shipped first. --- credit_overage_test.go | 33 +++++++++++++++++++++++++++++++++ models.go | 24 +++++++++++++++--------- rulecheck.go | 20 ++++++++++++++------ 3 files changed, 62 insertions(+), 15 deletions(-) diff --git a/credit_overage_test.go b/credit_overage_test.go index 21a633f..f6dea5a 100644 --- a/credit_overage_test.go +++ b/credit_overage_test.go @@ -31,6 +31,13 @@ func TestCreditOverage(t *testing.T) { return company } + companyWithCap := func(balance float64, cap float64) *rulesengine.Company { + enabled := true + company := companyWith(balance, &enabled) + company.CreditOverageCaps = map[string]float64{creditID: cap} + return company + } + creditRule := func() *rulesengine.Rule { rule := createTestRule() condition := createTestCondition(rulesengine.ConditionTypeCredit) @@ -116,4 +123,30 @@ func TestCreditOverage(t *testing.T) { require.NoError(t, err) assert.True(t, result.Value) }) + + // SCHX-582 cap: the floor moves from zero to -cap rather than disappearing. + t.Run("allows while inside the cap", func(t *testing.T) { + assert.True(t, matches(t, companyWithCap(-40, 100))) + }) + + t.Run("denies once the cap is spent", func(t *testing.T) { + assert.False(t, matches(t, companyWithCap(-100, 100))) + }) + + t.Run("denies past the cap", func(t *testing.T) { + assert.False(t, matches(t, companyWithCap(-140, 100))) + }) + + // A cap on one credit must not bound a different one. + t.Run("cap does not leak across credits", func(t *testing.T) { + company := companyWithCap(-140, 100) + company.CreditOverageCaps = map[string]float64{"other-credit": 100} + assert.True(t, matches(t, company)) + }) + + // Absent cap keeps the uncapped behaviour that shipped first. + t.Run("no cap means uncapped", func(t *testing.T) { + enabled := true + assert.True(t, matches(t, companyWith(-10_000, &enabled))) + }) } diff --git a/models.go b/models.go index 2ec50eb..c5e5915 100644 --- a/models.go +++ b/models.go @@ -178,15 +178,21 @@ type Company struct { // overage rate rather than being denied, so the balance stops gating the // check. Absent or false is the historical behaviour, so a caller that does // not send the field keeps hard-stopping at zero. - CreditOverageEnabled map[string]bool `json:"credit_overage_enabled"` - Entitlements JSONSlice[*FeatureEntitlement] `json:"entitlements,omitempty"` - Keys map[string]string `json:"keys"` - Metrics CompanyMetricCollection `json:"metrics"` - PlanIDs JSONSlice[string] `json:"plan_ids"` - PlanVersionIDs JSONSlice[string] `json:"plan_version_ids"` - Rules JSONSlice[*Rule] `json:"rules"` - Subscription *Subscription `json:"subscription"` - Traits JSONSlice[*Trait] `json:"traits"` + CreditOverageEnabled map[string]bool `json:"credit_overage_enabled"` + // CreditOverageCaps is the optional ceiling on overage per credit, in + // credits, keyed the same way. It only means anything where + // CreditOverageEnabled is true. Present means the balance may run down to + // -cap before the check denies; absent means uncapped, which is the + // behaviour that shipped before caps existed. + CreditOverageCaps map[string]float64 `json:"credit_overage_caps"` + Entitlements JSONSlice[*FeatureEntitlement] `json:"entitlements,omitempty"` + Keys map[string]string `json:"keys"` + Metrics CompanyMetricCollection `json:"metrics"` + PlanIDs JSONSlice[string] `json:"plan_ids"` + PlanVersionIDs JSONSlice[string] `json:"plan_version_ids"` + Rules JSONSlice[*Rule] `json:"rules"` + Subscription *Subscription `json:"subscription"` + Traits JSONSlice[*Trait] `json:"traits"` mu sync.Mutex `json:"-"` // mutex for thread safety } diff --git a/rulecheck.go b/rulecheck.go index 5363a59..9c57cbf 100644 --- a/rulecheck.go +++ b/rulecheck.go @@ -147,16 +147,24 @@ func (s *RuleCheckService) checkCreditBalanceCondition(ctx context.Context, scop } } - // SCHX-582: with overage enabled the balance no longer gates the check — - // consumption continues past zero and accrues at the configured rate, so - // every branch below (all of which compare against the balance) would ask - // the wrong question. There is no cap in the current design, so once overage - // is on there is nothing further to compare against. + // SCHX-582: with overage enabled the check fails open past zero — the + // balance keeps being drawn and accrues at the configured rate — so the + // branches below, which all compare against a positive balance, would ask + // the wrong question. + // + // The cap moves the floor rather than removing it: the balance is allowed + // to run down to -cap, and the check denies beyond that. An absent cap is + // uncapped, which is what shipped before caps existed. // // Mirrors check_credit_balance_condition in rulesengine-rust; the two must // agree (see SCHY-515) until the Go engine is retired. if scope.Company.CreditOverageEnabled[*condition.CreditID] { - return true, nil + overageCap, capped := scope.Company.CreditOverageCaps[*condition.CreditID] + if !capped { + return true, nil + } + + return creditBalance > -overageCap, nil } // Precedence on credit-balance conditions, most specific first. No From 6c7f04823cb464874828aa39dd604fc8310f6182 Mon Sep 17 00:00:00 2001 From: Christopher Brady Date: Thu, 3 Sep 2026 12:38:36 -0600 Subject: [PATCH 2/4] carry overage as one nullable map instead of two MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Overage has three states per credit — off, on-and-uncapped, on-and-capped — and the enabled-set plus cap-map spelling could express combinations that mean nothing: a cap on a credit that is not enabled, or enabled with no entry either side. Nothing prevented the two disagreeing. One map keyed by credit makes those unrepresentable: absent is off, a nil value is uncapped, and a set value is the cap. The check is one lookup rather than two. This replaces credit_overage_enabled from #30 rather than adding alongside it. Nothing consumes that field yet, so there is no reason to ship both and deprecate one later. --- credit_overage_test.go | 16 +++++++++++---- models.go | 45 ++++++++++++++++++++++-------------------- rulecheck.go | 7 +++---- 3 files changed, 39 insertions(+), 29 deletions(-) diff --git a/credit_overage_test.go b/credit_overage_test.go index f6dea5a..db0de16 100644 --- a/credit_overage_test.go +++ b/credit_overage_test.go @@ -22,11 +22,15 @@ func TestCreditOverage(t *testing.T) { const creditID = "test-credit-id" + // overage: nil leaves the credit out of the map entirely (off); non-nil puts + // it in, with the pointed-at value as the cap — nil cap meaning uncapped. companyWith := func(balance float64, overage *bool) *rulesengine.Company { company := createTestCompany() company.CreditBalances = map[string]float64{creditID: balance} - if overage != nil { - company.CreditOverageEnabled = map[string]bool{creditID: *overage} + if overage != nil && *overage { + company.CreditOverage = map[string]*float64{creditID: nil} + } else if overage != nil { + company.CreditOverage = map[string]*float64{} } return company } @@ -34,7 +38,7 @@ func TestCreditOverage(t *testing.T) { companyWithCap := func(balance float64, cap float64) *rulesengine.Company { enabled := true company := companyWith(balance, &enabled) - company.CreditOverageCaps = map[string]float64{creditID: cap} + company.CreditOverage = map[string]*float64{creditID: &cap} return company } @@ -140,7 +144,11 @@ func TestCreditOverage(t *testing.T) { // A cap on one credit must not bound a different one. t.Run("cap does not leak across credits", func(t *testing.T) { company := companyWithCap(-140, 100) - company.CreditOverageCaps = map[string]float64{"other-credit": 100} + otherCap := 100.0 + company.CreditOverage = map[string]*float64{ + creditID: nil, + "other-credit": &otherCap, + } assert.True(t, matches(t, company)) }) diff --git a/models.go b/models.go index c5e5915..f85e9c7 100644 --- a/models.go +++ b/models.go @@ -172,27 +172,30 @@ type Company struct { BasePlanID *string `json:"base_plan_id"` BillingProductIDs JSONSlice[string] `json:"billing_product_ids"` CreditBalances map[string]float64 `json:"credit_balances"` - // CreditOverageEnabled is the per-credit overage opt-in (SCHX-582), keyed by - // billing credit ID — the same key CreditBalances uses. When true for a - // credit, consumption continues past a zero balance and accrues against an - // overage rate rather than being denied, so the balance stops gating the - // check. Absent or false is the historical behaviour, so a caller that does - // not send the field keeps hard-stopping at zero. - CreditOverageEnabled map[string]bool `json:"credit_overage_enabled"` - // CreditOverageCaps is the optional ceiling on overage per credit, in - // credits, keyed the same way. It only means anything where - // CreditOverageEnabled is true. Present means the balance may run down to - // -cap before the check denies; absent means uncapped, which is the - // behaviour that shipped before caps existed. - CreditOverageCaps map[string]float64 `json:"credit_overage_caps"` - Entitlements JSONSlice[*FeatureEntitlement] `json:"entitlements,omitempty"` - Keys map[string]string `json:"keys"` - Metrics CompanyMetricCollection `json:"metrics"` - PlanIDs JSONSlice[string] `json:"plan_ids"` - PlanVersionIDs JSONSlice[string] `json:"plan_version_ids"` - Rules JSONSlice[*Rule] `json:"rules"` - Subscription *Subscription `json:"subscription"` - Traits JSONSlice[*Trait] `json:"traits"` + // CreditOverage is per-credit overage config (SCHX-582), keyed by billing + // credit ID — the same key CreditBalances uses. + // + // Three states, which is why the value is nullable rather than this being a + // map of caps or a map of bools: + // + // key absent -> overage off; an exhausted balance denies, as it always has + // value nil -> overage on, uncapped; the balance stops gating the check + // value set -> overage on, capped; the balance may run down to -cap + // + // One map rather than an enabled-set plus a cap-map because those two can + // disagree — a cap on a credit that is not enabled, or vice versa — and + // neither state means anything. + // + // A caller that does not send the field keeps hard-stopping at zero. + CreditOverage map[string]*float64 `json:"credit_overage"` + Entitlements JSONSlice[*FeatureEntitlement] `json:"entitlements,omitempty"` + Keys map[string]string `json:"keys"` + Metrics CompanyMetricCollection `json:"metrics"` + PlanIDs JSONSlice[string] `json:"plan_ids"` + PlanVersionIDs JSONSlice[string] `json:"plan_version_ids"` + Rules JSONSlice[*Rule] `json:"rules"` + Subscription *Subscription `json:"subscription"` + Traits JSONSlice[*Trait] `json:"traits"` mu sync.Mutex `json:"-"` // mutex for thread safety } diff --git a/rulecheck.go b/rulecheck.go index 9c57cbf..173854a 100644 --- a/rulecheck.go +++ b/rulecheck.go @@ -158,13 +158,12 @@ func (s *RuleCheckService) checkCreditBalanceCondition(ctx context.Context, scop // // Mirrors check_credit_balance_condition in rulesengine-rust; the two must // agree (see SCHY-515) until the Go engine is retired. - if scope.Company.CreditOverageEnabled[*condition.CreditID] { - overageCap, capped := scope.Company.CreditOverageCaps[*condition.CreditID] - if !capped { + if overageCap, overageOn := scope.Company.CreditOverage[*condition.CreditID]; overageOn { + if overageCap == nil { return true, nil } - return creditBalance > -overageCap, nil + return creditBalance > -*overageCap, nil } // Precedence on credit-balance conditions, most specific first. No From 31746a164cd0b0b571927f424adc41234d2d20d2 Mon Sep 17 00:00:00 2001 From: Christopher Brady Date: Thu, 3 Sep 2026 13:02:28 -0600 Subject: [PATCH 3/4] rename a test param off a predeclared identifier golangci-lint's predeclared linter flags 'cap' shadowing the builtin. --- credit_overage_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/credit_overage_test.go b/credit_overage_test.go index db0de16..8541650 100644 --- a/credit_overage_test.go +++ b/credit_overage_test.go @@ -35,10 +35,10 @@ func TestCreditOverage(t *testing.T) { return company } - companyWithCap := func(balance float64, cap float64) *rulesengine.Company { + companyWithCap := func(balance float64, limit float64) *rulesengine.Company { enabled := true company := companyWith(balance, &enabled) - company.CreditOverage = map[string]*float64{creditID: &cap} + company.CreditOverage = map[string]*float64{creditID: &limit} return company } From 67fdfeb04019bbce0c0188ecf416d7117331c30e Mon Sep 17 00:00:00 2001 From: Christopher Brady Date: Thu, 3 Sep 2026 14:46:30 -0600 Subject: [PATCH 4/4] measure the cap against the cost, not the balance alone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The overage branch returned before the cost/quantity precedence below it, so the cap was compared against the balance as it stood rather than as it would stand after the call. That enforced the cap only to within one call's cost: a company 5 credits short of a 100-credit cap passed a call costing 50 and landed 45 past it. Cost is now resolved first and overage shifts the floor it is measured against: balance + allowance >= cost. Uncapped still returns before the comparison, and with no overage the allowance is zero, which is exactly the balance >= cost check that has always applied. Adds the capped-plus-cost cases that were missing — every existing overage test used the legacy single-unit path, which is why this got through — and the serialization round-trip proving absent, present-and-null, and present-and-set stay distinct in both directions. --- credit_overage_serialization_test.go | 93 ++++++++++++++++++++++++++++ credit_overage_test.go | 46 ++++++++++++++ rulecheck.go | 60 +++++++++--------- 3 files changed, 167 insertions(+), 32 deletions(-) create mode 100644 credit_overage_serialization_test.go diff --git a/credit_overage_serialization_test.go b/credit_overage_serialization_test.go new file mode 100644 index 0000000..8c9fc64 --- /dev/null +++ b/credit_overage_serialization_test.go @@ -0,0 +1,93 @@ +package rulesengine_test + +import ( + "encoding/json" + "testing" + + "github.com/schematichq/rulesengine" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The overage map leans on a distinction JSON can express but is easy to lose in +// a hand-written client: a key present with a null value (overage on, uncapped) +// against the key being absent (overage off). Collapsing those turns "gate at +// zero" into "never gate", which is why it is asserted rather than assumed. +func TestCreditOverageSerialization(t *testing.T) { + const creditID = "test-credit-id" + + t.Run("decodes the three states distinctly", func(t *testing.T) { + for _, tc := range []struct { + name string + raw string + overageOn bool + uncapped bool + cap float64 + }{ + {name: "field absent", raw: `{}`}, + {name: "field null", raw: `{"credit_overage":null}`}, + {name: "map empty", raw: `{"credit_overage":{}}`}, + {name: "value null is on and uncapped", raw: `{"credit_overage":{"test-credit-id":null}}`, overageOn: true, uncapped: true}, + {name: "value set is on and capped", raw: `{"credit_overage":{"test-credit-id":100}}`, overageOn: true, cap: 100}, + } { + t.Run(tc.name, func(t *testing.T) { + var company rulesengine.Company + require.NoError(t, json.Unmarshal([]byte(tc.raw), &company)) + + overageCap, overageOn := company.CreditOverage[creditID] + require.Equal(t, tc.overageOn, overageOn, "presence in the map is the opt-in") + + if !tc.overageOn { + return + } + + if tc.uncapped { + assert.Nil(t, overageCap, "a null value must stay nil, not become zero") + return + } + + require.NotNil(t, overageCap) + assert.Equal(t, tc.cap, *overageCap) + }) + } + }) + + // Round-tripping must not quietly promote nil to 0. A zero cap denies every + // draw past the balance, which is the opposite of what nil means. + t.Run("round-trips without collapsing nil to zero", func(t *testing.T) { + limit := float64(100) + for name, overage := range map[string]map[string]*float64{ + "uncapped": {creditID: nil}, + "capped": {creditID: &limit}, + "empty": {}, + "nil map": nil, + } { + t.Run(name, func(t *testing.T) { + encoded, err := json.Marshal(rulesengine.Company{CreditOverage: overage}) + require.NoError(t, err) + + var decoded rulesengine.Company + require.NoError(t, json.Unmarshal(encoded, &decoded)) + + assert.Len(t, decoded.CreditOverage, len(overage)) + + want, wantOn := overage[creditID] + got, gotOn := decoded.CreditOverage[creditID] + require.Equal(t, wantOn, gotOn) + + if !wantOn { + return + } + + if want == nil { + assert.Nil(t, got, "nil cap must not decode as zero") + return + } + + require.NotNil(t, got) + assert.Equal(t, *want, *got) + }) + } + }) +} diff --git a/credit_overage_test.go b/credit_overage_test.go index 8541650..31591db 100644 --- a/credit_overage_test.go +++ b/credit_overage_test.go @@ -157,4 +157,50 @@ func TestCreditOverage(t *testing.T) { enabled := true assert.True(t, matches(t, companyWith(-10_000, &enabled))) }) + + // The cap has to bound the balance *after* this call, not before it. + // Checking the balance alone enforces the cap only to within one call's + // cost: 5 credits short of the cap, a call costing 50 would pass and land 45 + // past it. + matchesWithCost := func(t *testing.T, company *rulesengine.Company, cost float64) bool { + t.Helper() + + flag := createTestFlag() + // createTestFlag randomizes DefaultValue and CheckFlag falls back to it + // when no rule matches, so a true default would pass without the rule. + flag.DefaultValue = false + flag.Rules = []*rulesengine.Rule{creditRule()} + + result, err := rulesengine.CheckFlag( + ctx, company, nil, flag, rulesengine.WithCreditCost(creditID, cost), + ) + require.NoError(t, err) + return result.Value + } + + t.Run("denies a cost that would carry past the cap", func(t *testing.T) { + assert.False(t, matchesWithCost(t, companyWithCap(-95, 100), 50)) + }) + + t.Run("allows a cost that fits inside the cap", func(t *testing.T) { + assert.True(t, matchesWithCost(t, companyWithCap(-40, 100), 50)) + }) + + // Exactly reaching the cap is allowed; the next credit past it is not. + t.Run("allows a cost landing exactly on the cap", func(t *testing.T) { + assert.True(t, matchesWithCost(t, companyWithCap(-50, 100), 50)) + }) + + // Uncapped ignores the cost entirely — there is no floor to measure against. + t.Run("uncapped allows any cost", func(t *testing.T) { + enabled := true + assert.True(t, matchesWithCost(t, companyWith(-10_000, &enabled), 5_000)) + }) + + // Without overage the allowance is zero, so this stays the balance >= cost + // check that has always applied. + t.Run("without overage a cost still gates on balance", func(t *testing.T) { + assert.False(t, matchesWithCost(t, companyWith(10, nil), 50)) + assert.True(t, matchesWithCost(t, companyWith(60, nil), 50)) + }) } diff --git a/rulecheck.go b/rulecheck.go index 173854a..a5f3f41 100644 --- a/rulecheck.go +++ b/rulecheck.go @@ -147,49 +147,45 @@ func (s *RuleCheckService) checkCreditBalanceCondition(ctx context.Context, scop } } - // SCHX-582: with overage enabled the check fails open past zero — the - // balance keeps being drawn and accrues at the configured rate — so the - // branches below, which all compare against a positive balance, would ask - // the wrong question. + // What this call costs, most specific source first. No options supplied + // falls through to the legacy single-unit check. + // 1. creditCost[credit_id]: caller-supplied per-call cost in credits. + // 2. eventUsage, when its event_subtype matches the condition's: + // simulated quantity for this specific event. + // 3. usage: generic quantity (no event disambiguation). + // 4. Legacy: a single unit at the consumption rate. + cost := consumptionRate + if creditCost, ok := scope.creditCost[*condition.CreditID]; ok { + cost = creditCost + } else if eu := scope.eventUsage; eu != nil && condition.EventSubtype != nil && + eu.eventSubtype == *condition.EventSubtype && eu.quantity > 0 { + cost = float64(eu.quantity) * consumptionRate + } else if scope.usage != nil && *scope.usage > 0 { + cost = float64(*scope.usage) * consumptionRate + } + + // SCHX-582: overage moves the floor the cost is measured against, rather + // than skipping the comparison. Checking the balance alone would enforce the + // cap only to within one call: a company at -95 against a cap of 100 would + // pass a call costing 50 and land at -145. // - // The cap moves the floor rather than removing it: the balance is allowed - // to run down to -cap, and the check denies beyond that. An absent cap is - // uncapped, which is what shipped before caps existed. + // An uncapped grant returns before the comparison — there is no floor to + // measure against, and the company is free to run the balance as negative as + // it likes. With no overage the allowance is zero, which reduces this to the + // balance >= cost check that has always applied. // // Mirrors check_credit_balance_condition in rulesengine-rust; the two must // agree (see SCHY-515) until the Go engine is retired. + var overageAllowance float64 if overageCap, overageOn := scope.Company.CreditOverage[*condition.CreditID]; overageOn { if overageCap == nil { return true, nil } - return creditBalance > -*overageCap, nil - } - - // Precedence on credit-balance conditions, most specific first. No - // options supplied falls through to the legacy single-unit check. - // 1. creditCost[credit_id]: caller-supplied per-call cost in credits; - // gate on balance >= cost. - // 2. eventUsage, when its event_subtype matches the condition's: - // simulated quantity for this specific event; gate on - // balance >= quantity × consumption_rate. - // 3. usage: generic quantity (no event disambiguation); gate on - // balance >= quantity × consumption_rate. - // 4. Legacy: balance >= consumption_rate (single unit). - if cost, ok := scope.creditCost[*condition.CreditID]; ok { - return creditBalance >= cost, nil - } - - if eu := scope.eventUsage; eu != nil && condition.EventSubtype != nil && - eu.eventSubtype == *condition.EventSubtype && eu.quantity > 0 { - return creditBalance >= float64(eu.quantity)*consumptionRate, nil - } - - if scope.usage != nil && *scope.usage > 0 { - return creditBalance >= float64(*scope.usage)*consumptionRate, nil + overageAllowance = *overageCap } - return creditBalance >= consumptionRate, nil + return creditBalance+overageAllowance >= cost, nil } func (s *RuleCheckService) checkBillingProductCondition(ctx context.Context, company *Company, condition *Condition) (bool, error) {