Skip to content

fix: append default policies + break domain↔resource compile cycle#4

Merged
C-Sinclair merged 6 commits intomainfrom
review/fix-domain-merge-cycle-follow-up
Apr 21, 2026
Merged

fix: append default policies + break domain↔resource compile cycle#4
C-Sinclair merged 6 commits intomainfrom
review/fix-domain-merge-cycle-follow-up

Conversation

@C-Sinclair
Copy link
Copy Markdown

Supersedes #1 by @barnabasJ. Original commits preserved; one follow-up commit on top restores pipeline entries and naming that were inadvertently dropped in the original branch.

Summary

Two independent fixes surfaced while adopting ash_grant in a real codebase (ARCC Center — Ash 3.x, ~57 resources, domain-level code_interface blocks).

1. cfce2b4 — append default policies after user-defined ones

AshGrant.Transformers.AddDefaultPolicies was prepending its generated policies. Ash bypass policies only short-circuit policies that come after them, so user-defined bypass action_type(:create) blocks (e.g. "allow public registration for nil actor") never fired — ash_grant's permission check ran first and denied.

Flip the builder call to type: :append so user-defined policies (including bypasses) are evaluated first and can properly short-circuit ash_grant's generated checks.

2. e56b814 — break domain↔resource compile cycle with domain-level code_interface

Combining AshGrant on a resource with AshGrant.Domain + a code_interface do define … end block on the domain deadlocked at compile:

  • The resource's MergeDomainConfig transformer forced the domain to compile (to read domain-level ash_grant DSL).
  • The domain's code_interface transformer was waiting for the resource to finish compiling.

Fix:

  • Remove MergeDomainConfig transformer. Move resolver/scope merging from compile-time into runtime AshGrant.Info calls: resolver/1 falls back to the domain; scopes/1 merges resource + domain scopes with resource precedence.
  • Convert compile-time validators to Spark verifiers. ValidateResolverPresent and ValidateScopes become verifiers — they run post-compile and can safely reach into the domain without creating a cycle. Spark surfaces verifier errors as compile warnings, so AshGrant.Check.resolve_permissions/3 also raises a clear ArgumentError at authorization time if no resolver is configured.
  • Regression test. test/ash_grant/code_interface_cycle_test.exs pairs AshGrant on a resource with AshGrant.Domain + code_interface on the domain — this case deadlocked on the previous code.

3. f637749 — restore transformers and scope naming dropped during rebase

The previous two commits unintentionally dropped content already on main:

  • AshGrant.Transformers.AddArgumentResolvers was removed from the extension's transformer list. That transformer wires up every resolve_argument declaration; without it the DSL entity exists but silently no-ops.
  • AshGrant.Transformers.ValidateScopeThroughs was also removed, dropping compile-time validation for scope_through entities.
  • Moduledoc examples in lib/ash_grant.ex were rewritten :always:all, undoing the refactor: rename :all scope to :always across codebase jhlee111/ash_grant#84 rename in docs only.
  • The new AshGrant.Verifiers.ValidateScopes lost the instance_key attribute check that main's equivalent transformer did, and added an unrelated "scope :all is missing" warning that collides with main's :always convention.

This commit restores the two transformers in the pipeline, reverts the moduledoc examples, ports the instance_key validation into the new verifier, and drops the stray common-scope warning. Test fixtures that used :all switch to :always.

Test plan

  • mix test — 1062 tests, 1 failure, and that failure (default_field_policies_test.exs:52) reproduces on main unchanged, so it is pre-existing and unrelated.
  • 52/52 tests in the code-cycle regression suite and the domain-inheritance suite pass.
  • Compiles under mix compile (the expected "no resolver configured" warning on NoResolverPost is emitted by the verifier, as designed).

barnabasJ and others added 4 commits April 20, 2026 02:36
Ash bypass policies only short-circuit policies that come after them.
add_entity defaults to prepend, which placed AshGrant's generated
policies before user-defined bypass policies, making bypasses
ineffective.

Switch to type: :append so user-defined bypass policies are evaluated
first and can properly short-circuit AshGrant's permission checks.
…face

A resource using `AshGrant` plus a domain that uses `AshGrant.Domain` AND
declares a `code_interface do define … end` block deadlocked on compile:
the resource's `MergeDomainConfig` transformer forced the domain to compile,
while the domain's code_interface transformer was waiting for the resource.

Move domain config merging from a compile-time transformer to runtime
`AshGrant.Info` calls (`resolver/1` falls back to the domain, `scopes/1`
merges resource + domain scopes with resource precedence). Convert the
compile-time presence checks to Spark verifiers, which run post-compile
and can safely reach into the domain. Spark surfaces verifier errors as
compile warnings, so `AshGrant.Check.resolve_permissions/3` also raises a
clear `ArgumentError` when no resolver is configured at authorization time.

Adds a regression test (`code_interface_cycle_test.exs`) that pairs
`AshGrant` on a resource with `AshGrant.Domain` + `code_interface` on the
domain. This case deadlocked on the previous code.
The previous two commits unintentionally dropped content that is on
team-alembic/ash_grant:main:

- AshGrant.Transformers.AddArgumentResolvers was removed from the
  extension's transformer list. That transformer wires up every
  `resolve_argument` declaration; without it the DSL entity exists but
  silently no-ops, breaking any resource that relies on argument-based
  scopes.
- AshGrant.Transformers.ValidateScopeThroughs was also removed from the
  list, dropping compile-time validation for scope_through entities.
- Moduledoc examples in lib/ash_grant.ex were rewritten `:always` →
  `:all`, undoing the jhlee111#84 rename in docs only (runtime was unchanged,
  but the docs drifted from the rest of the codebase).
- AshGrant.Verifiers.ValidateScopes lost the `instance_key` attribute
  check that main's equivalent transformer did, and added an unrelated
  "scope `:all` is missing" warning that collides with main's `:always`
  convention.

Restore the two transformers in the extension's pipeline, revert the
moduledoc examples to `:always`, port main's `instance_key` validation
into the new verifier, and drop the stray common-scope warning. Test
fixtures that used `:all` switch to `:always`.

The intended fixes — append policies, runtime domain merge, verifier
conversion, runtime resolver guard — are preserved.
@C-Sinclair C-Sinclair merged commit f526217 into main Apr 21, 2026
1 check passed
C-Sinclair added a commit that referenced this pull request Apr 21, 2026
Resolves conflicts from two changes landed on main:

- #2 removed scope inheritance (no impact here — grants don't use scope
  inheritance; tests still combine conditions inline with `and`).
- #4 broke the compile-time cycle between resource and domain by removing
  the MergeDomainConfig transformer and the ValidateResolverPresent
  transformer, moving resolver-merge logic into AshGrant.Info.resolver/1
  at runtime and turning ValidateResolverPresent into a verifier.

Adjustments to this branch:

- Dropped lib/ash_grant/transformers/validate_resolver_present.ex — the
  file was deleted on main (replaced by a verifier). My earlier explicit
  before?/after? guards in that file were no longer needed.
- Reintegrated NormalizeGrants and SynthesizeGrantsResolver into the
  new transformer list (main-first ordering, no references to the now-gone
  MergeDomainConfig).
- Added ValidateGrantReferences to the verifier list alongside the new
  ValidateResolverPresent and ValidateScopes verifiers.
- Cleaned before?/after? in NormalizeGrants and SynthesizeGrantsResolver
  to reference only transformers that still exist.

Full suite still green (1089 tests, 1 pre-existing unrelated failure).
Credo clean on all new files.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants