docs and fix: Clarify that yaml priority only applies to role_extensions

CHANGELOG.rst

@@ -32,8 +32,9 @@ Added
   removing roles that still have assignments, and repeatable ``--dir`` for CI and local runs.
 * Added ``role_extensions`` support: an application or deployment can add or remove permissions and
   replace the display metadata or ``hidden`` flag of an existing static role without copying its
-  definition. ``priority`` resolves conflicts; an unresolvable equal-priority conflict stops the run
-  before any database change (ADR 0023).
+  definition. ``priority`` resolves conflicts between extensions; an unresolvable equal-priority
+  conflict stops the run before any database change (ADR 0023). Priority does not resolve conflicting
+  base definitions (ADR 0031).
 * Added first-class tables for compiled definitions and their provenance, in migration
   ``0011_authz_schema_definitions``: permission categories, permission definitions, role
   definitions, role-permission grants, schema sources, and one source-link table per definition kind
@@ -51,6 +52,15 @@ Added
   ``openedx_authz/engine/schema/paragon_icons.py`` and regenerated with
   ``make paragon_icons`` (ADR 0029).
 
+Fixed
+=====
+
+* The schema compiler no longer resolves conflicting base definitions by ``priority``. Two files
+  that declare the same permission category, permission, or role with differing content are now a
+  conflict regardless of priority, matching schema validation; only ``role_extensions`` are resolved
+  by priority. This aligns the compiler with ADR 0031 and removes a path where a higher-priority file
+  could silently replace a role's base definition.
+
 Notes
 =====
 

docs/decisions/0017-static-authorization-schema.rst

@@ -19,7 +19,7 @@ Decision
 1. Schema format and boundary
 =============================
 
-The authz schema is a versioned YAML format for static permissions, permission categories, roles, and changes to existing roles. Every file declares ``schema_version`` and ``priority``, which apply to all definitions in that file. Priority is used when role extensions conflict. Open edX publishes a :download:`machine-readable JSON Schema <../../src/openedx_authz/schema/authz-schema-v1.json>` for this format so that editors, CI, and the compiler all apply the same field and validation rules.
+The authz schema is a versioned YAML format for static permissions, permission categories, roles, and changes to existing roles. Every file declares ``schema_version`` and ``priority``, which apply to all definitions in that file. Priority is used only when role extensions conflict; it does not resolve conflicting base definitions (see `ADR 0031`_). Open edX publishes a :download:`machine-readable JSON Schema <../../src/openedx_authz/schema/authz-schema-v1.json>` for this format so that editors, CI, and the compiler all apply the same field and validation rules.
 
 The existing static role and permission definitions in Python modules and ``authz.policy`` will move into the schema. Once this migration is complete, the schema becomes the source for static definitions, so developers add a new role or permission there without duplicating it in Python constants or policy files.
 
@@ -97,7 +97,7 @@ In these examples, ``courses.view_course`` appears under ``course_content`` in t
 
    p, role^course_observer, act^courses.view_course, course-v1^*, allow
 
-The compiler also creates a row that links ``course_admin`` to ``courses.delete_course``. Priority resolves conflicts between role extensions; it does not apply to individual permissions or control how roles or permissions appear in the UI.
+The compiler also creates a row that links ``course_admin`` to ``courses.delete_course``. Priority resolves conflicts between role extensions; it does not resolve conflicting base definitions (`ADR 0031`_), does not apply to individual permissions, and does not control how roles or permissions appear in the UI.
 
 The schema contains static definitions, while user assignments and user-defined roles stay in the application database. Casbin's ``model.conf`` and matcher also remain owned by ``openedx-authz``.
 
@@ -119,7 +119,7 @@ After loading every file, validation checks the combined definitions. It rejects
 * references to permissions or categories that do not exist;
 * a role extension whose role does not exist in the combined definitions;
 * a role used in a scope where one of its permissions cannot apply;
-* conflicting static definitions;
+* conflicting static (base) definitions, regardless of their priority (`ADR 0031`_);
 * conflicting role extensions with the same priority; and
 * unsupported combinations of schema versions.
 
@@ -160,12 +160,14 @@ References
 
 * `ADR 0016`_
 * `ADR 0023`_
+* `ADR 0031`_
 * `Casbin adapters`_
 * `JSON Schema`_
 * `Paragon icons`_
 
 .. _ADR 0016: 0016-static-and-dynamic-roles.rst
 .. _ADR 0023: https://github.com/openedx/openedx-authz/pull/430
+.. _ADR 0031: 0031-priority-applies-to-role-extensions-only.rst
 .. _Casbin adapters: https://v3.casbin.org/docs/adapters
 .. _JSON Schema: https://json-schema.org/draft/2020-12/json-schema-core
 .. _Paragon icons: https://paragon-openedx.netlify.app/components/icon/

docs/decisions/0023-extend-static-roles.rst

@@ -56,7 +56,7 @@ Setting ``hidden: true`` removes the role from the normal API results used to di
 
 The compiler resolves extensions after it loads every static role and permission. It rejects an extension when the target role or one of the permissions does not exist. Adding a permission that the role already has or removing one it does not have produces a warning and leaves the result unchanged.
 
-Several applications or deployment files may extend the same role. Changes to different fields are combined, while priority resolves changes to the same metadata field or permission. If two contributions with the same priority disagree, validation stops before the database changes.
+Several applications or deployment files may extend the same role. Changes to different fields are combined, while priority resolves changes to the same metadata field or permission. If two contributions with the same priority disagree, validation stops before the database changes. Priority resolves conflicts between extensions only; it does not let an extension or a competing base definition replace a role's base definition, which stays single-source-of-truth (`ADR 0031`_).
 
 4. Tutor patch
 ==============
@@ -131,10 +131,12 @@ References
 * `ADR 0017`_
 * `ADR 0018`_
 * `ADR 0019`_
+* `ADR 0031`_
 * :ref:`Authorization Schema Reference`
 * `Tutor plugin development`_
 
 .. _ADR 0017: 0017-static-authorization-schema.rst
 .. _ADR 0018: 0018-authorization-schema-lifecycle.rst
 .. _ADR 0019: 0019-authorization-schema-discovery.rst
+.. _ADR 0031: 0031-priority-applies-to-role-extensions-only.rst
 .. _Tutor plugin development: https://docs.tutor.edly.io/plugins/v0/gettingstarted.html

docs/decisions/0031-priority-applies-to-role-extensions-only.rst

@@ -0,0 +1,106 @@
+0031: Priority Applies Only to Role Extensions, Not Base Definitions
+####################################################################
+
+Status
+******
+
+**Draft**
+
+Context
+*******
+
+The authorization schema is contributed as YAML files. Every file declares a
+``priority`` number alongside its ``schema_version``, and that number applies to
+all definitions in the file. `ADR 0017`_ and `ADR 0023`_ introduced ``priority``
+for one purpose: to resolve conflicts between ``role_extensions``. When several
+files change the same metadata field or the same permission on the same existing
+role, the highest priority wins; an equal-priority disagreement is rejected.
+
+Because ``priority`` sits at the top of every file and visually governs
+everything below it, the YAML structure invites a reasonable but incorrect
+assumption: that priority also decides a winner when two files declare the same
+*base definition* differently. This ADR records that it does not, and explains
+why, so the intent is visible to anyone reading the schema format later.
+
+Two kinds of contribution matter here, and the distinction is the whole point:
+
+* A **base definition** is a complete, self-contained declaration of a
+  permission category, a permission, or a role. It is the single source of truth
+  for that thing: a role's base definition *is* its full set of granted
+  permissions and its display metadata.
+* A **role extension** is an explicit, partial change to an existing base role.
+  It adds or removes individual permissions, or replaces individual display
+  fields. Priority exists to decide which change wins when two extensions touch
+  the same field or the same permission.
+
+Decision
+********
+
+Priority applies only to ``role_extensions``. It does **not** resolve conflicts
+between base definitions.
+
+Two files that declare the same permission category, permission, or role
+identifier with differing content are a conflict, and validation rejects that
+conflict as an error no matter what priorities the files declare. (Two files that
+declare the identical base definition are not a conflict; that remains a
+harmless duplicate.) Resolving such a disagreement is a deliberate authoring
+decision, not something a priority number should settle silently.
+
+We reach this decision for three reasons:
+
+1. **Safety of access-control definitions.** A base definition is a complete
+   grant of access. If priority could pick a winner, one file could silently
+   replace another file's entire role — including the full set of permissions it
+   grants — and the replaced definition would simply have no effect. A change
+   that broadens or narrows what a role can do should be explicit and reviewable,
+   never the quiet side effect of one file declaring a larger number than
+   another.
+
+2. **No need for it.** Deployments and applications adapt existing roles through
+   ``role_extensions``, which exist precisely so a contributor can change a role
+   without owning and restating its full definition (`ADR 0023`_). There is no
+   requirement for two files to each fully redefine the same base definition and
+   have one of them win.
+
+3. **Base definitions stay a single source of truth.** A category, permission,
+   or role is defined once; extensions are the mechanism for modifying it.
+   Keeping that boundary sharp preserves clear provenance for every granted
+   permission (`ADR 0025`_) and keeps the extension mechanism meaningful rather
+   than redundant.
+
+There is also a conceptual mismatch that makes "apply the same priority rule to
+base definitions" less tidy than it sounds. Extension resolution is granular: it
+compares one field or one permission at a time, so two files can change different
+parts of the same role and both take effect. A base definition has no such
+granularity — it would have to be replaced as a whole. Priority over base
+definitions would therefore be an all-or-nothing override, which is exactly the
+behavior the first reason argues against.
+
+Wherever ``priority`` is documented, the text states that it governs role
+extensions only, so a reader of the YAML format does not infer that it also
+ranks competing base definitions.
+
+Consequences
+************
+
+* Base definitions remain single-source-of-truth. A disagreement between two
+  base definitions of the same category, permission, or role is surfaced as an
+  error for an author to resolve, rather than resolved implicitly by priority.
+* To adapt an existing role, contributors use ``role_extensions`` rather than a
+  second, higher-priority base definition of the same role.
+* Documentation of the ``priority`` field states plainly that it applies to role
+  extensions only.
+* If a future need ever arises to replace a whole base definition from a separate
+  file, it will be introduced as its own explicitly designed and reviewable
+  mechanism, not as an implicit consequence of priority.
+
+References
+**********
+
+* `ADR 0017`_
+* `ADR 0023`_
+* `ADR 0025`_
+
+.. _ADR 0017: 0017-static-authorization-schema.rst
+.. _ADR 0023: 0023-extend-static-roles.rst
+.. _ADR 0025: 0025-authorization-schema-source-tracking.rst

docs/references/authorization-schema.rst

@@ -84,9 +84,9 @@ The version of the YAML format used by the file. Write it as a quoted ``major.mi
 ``priority``
 ============
 
-An integer used when several files extend the same role and change the same field or permission. A higher number takes precedence. Contributions with the same priority may be combined when they agree or affect different fields, but conflicting values at the same priority fail validation.
+An integer used only when several files extend the same role through ``role_extensions`` and change the same field or permission. A higher number takes precedence. Contributions with the same priority may be combined when they agree or affect different fields, but conflicting values at the same priority fail validation.
 
-Priority does not control the order shown in a user interface. Clients may sort roles, permissions, and categories for their own presentation.
+Priority applies to role extensions only. It does not resolve conflicting base definitions: two files that declare the same permission category, permission, or role ID with different content are rejected regardless of their priorities. Priority also does not control the order shown in a user interface. Clients may sort roles, permissions, and categories for their own presentation.
 
 ``permission_categories``
 =========================

src/openedx_authz/engine/schema/compilation.py

@@ -112,10 +112,11 @@ def compile(self, documents: list[SchemaDocument]) -> CompiledSchema:
         )
 
     def _collect(self, documents: list[SchemaDocument], kind: DefinitionKind, key) -> dict[str, _Tracked]:
-        """Gather base definitions keyed by identifier, resolving by priority.
+        """Gather base definitions keyed by identifier.
 
-        Higher priority wins on conflict; equal priority with differing content
-        raises; identical duplicates merge their sources.
+        Identical duplicates merge their sources; differing definitions for the
+        same identifier are a conflict and raise, regardless of priority
+        (ADR 0031). Priority applies only to role extensions.
         """
         tracked: dict[str, _Tracked] = {}
         for document in documents:
@@ -130,7 +131,7 @@ def _collect(self, documents: list[SchemaDocument], kind: DefinitionKind, key) -
                     )
                     continue
 
-                tracked[identifier] = self._resolve_priority(
+                tracked[identifier] = self._merge_base_definition(
                     kind.label,
                     identifier,
                     existing=existing,
@@ -142,49 +143,34 @@ def _collect(self, documents: list[SchemaDocument], kind: DefinitionKind, key) -
                 )
         return tracked
 
-    def _resolve_priority(
+    def _merge_base_definition(
         self,
         kind: str,
         identifier: str,
         *,
         existing: _Tracked,
         incoming: _Tracked,
     ) -> _Tracked:
-        """Resolve two competing definitions for one identifier by priority.
-
-        Single responsibility: decide which of two :class:`_Tracked` entries for
-        the same identifier survives.
-
-        * Identical definitions merge their sources (both files contributed the
-          same thing).
-        * Otherwise higher priority wins; the loser is warned about (ADR 0017
-          §4) so a valid-but-ineffective file does not look like it took effect.
-        * Equal priority with disagreeing definitions is unresolvable and raises
-          :class:`SchemaCompileError` so deployment stops before the database
-          changes.
-
-        Each ``_Tracked`` carries a single source; ``existing`` and ``incoming``
-        are symmetric, so this also serves conflicts between extension
-        contributions where only priority (not load order) decides the winner.
+        """Combine two base definitions for the same identifier.
+
+        A base definition is a single source of truth, so ``priority`` does not
+        pick a winner between two of them (ADR 0031).
+
+        * Identical definitions merge their sources: both files declared the
+          same thing, so the result records both as contributors.
+        * Differing definitions are an unresolvable conflict and raise
+          :class:`SchemaCompileError`, regardless of priority, so deployment
+          stops before the database changes. Whole-set validation rejects this
+          case first; raising here keeps the compiler safe on a direct call.
         """
         if existing.definition == incoming.definition:
             return replace(existing, sources=existing.sources + incoming.sources)
-        if incoming.priority == existing.priority:
-            raise SchemaCompileError(
-                f"Conflicting {kind} definition for {identifier!r} at equal priority "
-                f"{incoming.priority} "
-                f"({existing.sources[0].source_id} vs {incoming.sources[0].source_id})."
-            )
-        winner, loser = (incoming, existing) if incoming.priority > existing.priority else (existing, incoming)
-        self._warn_discarded(
-            kind,
-            identifier,
-            loser=loser.sources[0],
-            loser_priority=loser.priority,
-            winner=winner.sources[0],
-            winner_priority=winner.priority,
+        raise SchemaCompileError(
+            f"Conflicting {kind} definition for {identifier!r} "
+            f"({existing.sources[0].source_id} vs {incoming.sources[0].source_id}). "
+            f"Priority does not resolve base definitions (ADR 0031); "
+            f"use role_extensions to adapt an existing role."
         )
-        return winner
 
     @staticmethod
     def _warn_discarded(
@@ -390,10 +376,11 @@ def _resolve_contributions_based_on_priority(
     ) -> tuple[object, int, list[SourceRecord]]:
         """Pick the winning value among competing extension contributions.
 
-        The batch counterpart to :meth:`_resolve_priority`: where that method
-        decides between two base definitions pairwise, this decides among any
-        number of ``(value, priority, source)`` contributions to the same
-        extension field or permission.
+        This resolves extension contributions, where priority does decide a
+        winner (ADR 0023). It picks among any number of
+        ``(value, priority, source)`` contributions to the same extension field
+        or permission. Base definitions are handled separately by
+        :meth:`_merge_base_definition`, which does not use priority (ADR 0031).
 
         The highest priority wins. Every contribution in the top-priority group
         must agree; if they do not, ``on_tie(top_values)`` builds the message

src/openedx_authz/schema/authz-schema-v1.json

@@ -16,7 +16,7 @@
     },
     "priority": {
       "title": "priority",
-      "description": "The precedence used when contributions extend the same role. A higher number takes precedence when extensions conflict.",
+      "description": "The precedence used only when role_extensions contributions change the same role. A higher number takes precedence when extensions conflict. Priority does not resolve conflicting base definitions (permission categories, permissions, or roles); conflicting base definitions are rejected regardless of priority.",
       "type": "integer",
       "examples": [100]
     },