feat(reconcile): unify custom and predefined role handling under one field model

[rohilsurana]

What

Collapses the Role kind's two code paths (custom roles and predefined roles) into one field
model, part of adopting rules v2. A role's managed fields now follow a single rule: a field
written in the file is the whole desired value; an omitted field takes the role's default;
an explicitly empty field means empty. Custom and predefined roles differ only in their
default (empty for custom, the shipped definition for predefined) and in that a predefined
role is never created or deleted by the flow.

Why

Role was the most complex kind. The old code had a separate diffPredefinedRole with four
carve-out conditions, keep-if-omitted guards on custom roles, and three empty-list
rejections, all to protect the export round trip. Measured effect of the unification on
role.go:

• branch keywords 42 -> 15, compound conditions (&&/||) 13 -> 1
• diffPredefinedRole (53 lines) deleted, folded into one diffRoles
• validateRoleSpec 24 lines / 4 rules -> ~14 lines / 3 rules (name, no predefined delete, at least one permission)
• production code (role.go + reconciler) down ~50 lines

It also closes Role's two gaps on the rules-v2 compatibility audit (Rule 2, the field model,
and Rule 5, the export round trip).

The model

• roleDefault(name) returns the default fields: the shipped definition for a predefined
  role, empty for a custom role.
• resolve lays the spec's present fields over the default; diffFields reports what
  changed. One path for both role types.
• Presence tracking uses pointers (*string, *[]string) so an omitted field and an
  explicitly empty field are distinguishable, and so export can write scopes: [] for a
  field that is empty on the server. This removes the carve-outs entirely; the round trip
  was verified over the tricky reachable state (a predefined field emptied on the server).

Behavior changes to know

• Omitting a field on a role now means its default, not "keep the server value." For a custom
  role an omitted title clears it; for a predefined role an omitted field converges to the
  definition (unchanged from before). Files generated by frontier export write every
  managed field, so the normal edit-the-export workflow is unaffected; only a hand-trimmed
  file behaves differently.
• An unlisted predefined role with an empty legacy field now converges to its definition
  instead of staying frozen. More predictable, and export keeps such roles listed so the
  round trip never triggers a surprise.
• Empty title, description, and scopes are legal explicit values now: write scopes: [] to
  blank scopes, and an omitted custom field clears it. Permissions are the exception. The
  server rejects a role with no permissions, so a role must still resolve to at least one.
  A custom role with omitted or permissions: [], or a predefined role set to [], fails
  the plan up front with a clear message rather than late at the API. (This guard was briefly
  dropped in an earlier revision of this PR and is restored.)

Testing

• go build, go test ./internal/reconcile/..., go vet, gofmt, golangci-lint all
  pass. Test updates were mostly mechanical (pointer literals), plus new cases: an omitted
  custom title clears, a predefined field emptied on the server round-trips, and a role that
  resolves to no permissions (a custom role with omitted or empty permissions, or a predefined
  role set to []) fails the plan.

Status

RFC 0001 now describes this one field model (#1751), so this PR implements
what the RFC specifies. It targets main, now that the Validate interface change (#1776)
it built on has merged. The branch is rebased on the current main and merges cleanly. The
framework's "validate every document before applying" commit that this branch used to carry
has since landed in main on its own, so it is dropped here. The branch now holds just the
two role commits: the field-model unification and the no-permissions plan-time guard.

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
frontier	Ready	Preview, Comment	Jul 22, 2026 10:48am

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 47e03037-8630-4765-81d0-91fe209054bc

📥 Commits

Reviewing files that changed from the base of the PR and between 34fdba8 and da32e66.

📒 Files selected for processing (4)

• internal/reconcile/role.go
• internal/reconcile/role_reconciler.go
• internal/reconcile/role_reconciler_test.go
• internal/reconcile/role_test.go

🚧 Files skipped from review as they are similar to previous changes (4)

• internal/reconcile/role_reconciler.go
• internal/reconcile/role_reconciler_test.go
• internal/reconcile/role_test.go
• internal/reconcile/role.go

---

📝 Walkthrough

Summary by CodeRabbit

• New Features

  • Role configurations now distinguish omitted fields from explicitly empty values (including permissions and scopes).
  • Role exports emit only fields that differ from defaults, while preserving explicit empty values for accurate round-tripping.
  • Predefined roles are automatically restored to default settings when no longer customized.

• Bug Fixes

  • Improved role comparison by normalizing permissions and sorting scopes to avoid unnecessary updates.
  • Metadata is preserved during updates, while managed description metadata is correctly cleared when desired.

• Tests

  • Updated role reconciler and diff tests to cover the new omission/empty semantics and legacy empty round-tripping behavior.

Walkthrough

Role reconciliation now tracks YAML field presence with pointers, resolves desired roles over defaults, compares normalized fields, accounts for server roles, exports explicit differences, and applies operations using resolved role data.

Changes

Role reconciliation

Layer / File(s)	Summary
Role fields and resolution internal/reconcile/role.go	RoleSpec distinguishes omitted fields from explicit values; role defaults, current roles, diffs, and operations use normalized roleFields.
Role diff planning internal/reconcile/role.go, internal/reconcile/role_test.go	Validation and planning resolve specs over defaults, handle predefined and custom roles, reject empty permissions, and test explicit versus omitted fields.
Export and apply resolved roles internal/reconcile/role_reconciler.go, internal/reconcile/role_reconciler_test.go	Export preserves differing empty values, while apply builds requests from resolved fields and maintains metadata behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

• raystack/frontier#1737: Both changes modify role reconciliation and its RoleSpec, diffing, export, and apply behavior.

Suggested reviewers: whoabhisheksah

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

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 29913290645

Coverage decreased (-0.07%) to 46.209%

Details

• Coverage decreased (-0.07%) from the base build.
• Patch coverage: 109 of 109 lines across 2 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:	38538
Covered Lines:	17808
Line Coverage:	46.21%
Coverage Strength:	14.07 hits per line

---

💛 - Coveralls