fix(reconcile): honor a product's stated behavior on create and match feature-name case on apply

[rohilsurana]

What

Two ways the BillingProduct file's stated intent was not what got applied:

1. Behavior on create. Service.Create forced behavior = credits whenever credit_amount > 0, ignoring the behavior the file set. Now an explicit behavior is honored; only an omitted behavior on a credit product falls back to credits (the server's create-time default). The reconciler's diff mirrors the same rule, so a file that names a behavior the product was not created with fails the plan instead of being silently overridden.

2. Feature name case. The diff compared feature names lowercased and trimmed, but the apply sent them raw. Since the server looks features up by name, a case difference read as "no change" yet could fork a duplicate feature on a co-occurring update. Both sides now use one normalizeFeatureName helper.

Why

RFC 0001 Rule 2: the file is the desired state. The applied result must match what the file stated, with no silent override and no duplicate created behind a "no change" plan.

Testing

• New TestService_Create cases: an explicit behavior is honored on a credit product, and an omitted behavior defaults to credits.
• Reworked the diff behavior test and added a body test asserting feature names are normalized on apply.
• billing/... and internal/reconcile pass.

Stack

Stacked on fix/reconcile-permission-spicedb-grammar.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
frontier	Ready	Preview	Aug 18, 2026 6:16am

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c994c11d-5971-4cb2-a62d-92507e9f21e6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coveralls]

Coverage Report for CI Build 32106206559

Coverage increased (+0.01%) to 48.782%

Details

• Coverage increased (+0.01%) from the base build.
• Patch coverage: 11 of 11 lines across 3 files are fully covered (100%).
• No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.

---

Coverage Stats

Relevant Lines:	40099
Covered Lines:	19561
Line Coverage:	48.78%
Coverage Strength:	15.7 hits per line

---

💛 - Coveralls

[rohilsurana]

Review: honor behavior on create, match feature-name case on apply (RFC 0001 Rule 2)

Verified on this branch: go build ./..., go test ./internal/reconcile/... ./billing/product/..., and golangci-lint run on the changed packages all pass.

Both claims hold. Case normalization is now one shared function normalizeFeatureName (billingproduct.go:305), used by the diff (billingFeatureSetsEqual) and by apply (billingProductBody, billingproduct_reconciler.go:293), so apply cannot disagree with what the plan showed. The behavior fix captures statedBehavior before defaults (billing/product/service.go:67) and only folds to credits when the behavior is omitted, and the diff's expectedBehavior fold matches. I traced every create/diff/export combination and Rule 4 and Rule 5 both hold.

I also checked whether the same raw-case-on-apply gap remains for price names. It does not bite, because the billing service lowercases price names server-side on create and update (billing/product/service.go:456 and :518), unlike features. So leaving prices untouched is correct.

Verdict: approve with nits.

Nit:

1. The fix makes the diff and apply agree with each other, but not with the server's stored case. A pre-existing mixed-case feature (created via the CreateFeature API, or by the reconciler before this fix) still forks a duplicate when a co-occurring product change triggers an update. Concrete case: the server holds feature Foo on product tok, the file lists foo and also edits the description. The plan prints update product tok (description) with no feature change, but apply sends foo, the case-sensitive server lookup misses Foo, and a second foo row is created. This is narrow (legacy or externally created data) and a strict improvement over the old symmetric risk. A complete fix needs the server to canonicalize feature names on store and lookup.

[rohilsurana]

Follow-up on the nit.

Agreed, and left as is on purpose. This PR makes the diff and the apply agree on feature-name case, which removes the symmetric fork risk. The residual case, a legacy or externally created mixed-case feature row, needs the server to canonicalize feature names on store and on lookup. That is a server change outside this reconcile PR, so it is a follow-up, not part of this change.

[rohilsurana]

Re-review (head 75aa967)

The head moved only because of the rebase onto #1889. #1890's own patch is byte-for-byte identical to the previous review, so nothing in this PR's content changed. Build, tests, and lint are all green.

Re-confirmed: feature-name case is normalized by one shared normalizeFeatureName on both the diff path and the apply path, so apply cannot disagree with the plan. The behavior-on-create fold matches the diff, and the export round-trip plans zero ops.

The residual mixed-case feature fork is still open and needs a server-side feature-name canonicalization (UpsertFeature stores the name raw, unlike prices which the service lowercases). That is out of scope for this PR.

Verdict: approve with nits.