From b206a66b468034149bbb56e37d60c97596dcfd8d Mon Sep 17 00:00:00 2001 From: Matt de Courcelle Date: Sun, 17 May 2026 21:29:50 -0500 Subject: [PATCH] fix(ash_ui validations): trust `custom:*` opaque widget types in :list binding allow-list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Authoring validator (`validate_element_binding_locality!` at lib/ash_ui/resources/validations/authoring.ex:373) raises ArgumentError "does not expose collection semantics" for any :list-typed binding on Elements whose canonical widget_type isn't in @list_widgets ("list", "table", "info_list", "select", "list_repeat"). `custom:*` opaque widget types are application-extension primitives outside the canonical catalog (per Storage.canonical_widget_type at lib/ash_ui/dsl/storage.ex:162). The validator can't know their collection semantics and shouldn't gate the :list binding allow-list on them; trust the authoring intent. Conservative scope: - Bypass applies ONLY to "custom:*" string widget_types - nil widget_type (`ui_element` block missing entirely) still raises through downstream missing-element error paths - Canonical list-supporting widgets (`list`, `table`, etc.) remain the exclusive allow-list for non-custom Elements; row-template primitives like `:artifact_row` continue to belong INSIDE a `:list_repeat` parent that owns the list binding (per the Phase31RepeatListElement canonical pattern + Codex P2 #2 review) Triggered by Wave AshUI-3.9 Item 2 dispatch (2026-05-17/18). The original sizing investigation proposed adding `artifact_row` to @list_widgets so destination Elements could hold :list bindings directly; Codex P2 #2 review correctly pointed out that :list_repeat is the canonical destination wrapper (not :artifact_row) per the Phase31RepeatListElement test fixture. Per umbrella CLAUDE.md "substrate gaps become our roadmap, not asks-of-Pascal" — the `custom:*` bypass DOES belong here since the canonical catalog has no opinion on opaque application types. Per Codex P2 #3 review: the bypass is tightened to ONLY match `"custom:*"` binary strings. Earlier draft included `is_nil(widget_type)` which would have silently accepted bindings on Elements missing their `ui_element` declaration entirely; that case should still surface as a downstream missing-element error. Test coverage: - Existing negative test (`:text` widget rejecting :list) still passes (regression check that the allow-list still gates unknown-bad types) - New positive test: `"custom:thread_card"` widget compiles with `binding_type :list` The positive test uses a helper `assert_validator_accepts_list_binding` that checks the SPECIFIC validator error is not raised; downstream Ash domain-registration exceptions (which fire only AFTER the validator we're testing passes) are acknowledged as proof the validator accepted. Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/ash_ui/resources/validations/authoring.ex | 13 +++- test/ash_ui/resource/locality_test.exs | 65 +++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/lib/ash_ui/resources/validations/authoring.ex b/lib/ash_ui/resources/validations/authoring.ex index 88dbf341..7c78986b 100644 --- a/lib/ash_ui/resources/validations/authoring.ex +++ b/lib/ash_ui/resources/validations/authoring.ex @@ -370,7 +370,18 @@ defmodule AshUI.Resources.Validations.Authoring do "binding #{inspect(Map.get(binding, :id))} targets #{inspect(target)}, which is reserved for screen-scoped bindings" end - if binding_type == :list and not MapSet.member?(@list_widgets, widget_type) do + # `custom:*` widget types are application-extension primitives outside the + # canonical catalog; the validator can't know their collection semantics + # and shouldn't gate the :list binding allow-list on them. Trust the + # authoring intent for these explicitly-tagged opaque types. nil + # widget_type (`ui_element` block missing entirely) does NOT get the + # bypass — that should still surface as a downstream missing-element + # error rather than be silently accepted here. + custom_widget? = + is_binary(widget_type) and String.starts_with?(widget_type, "custom:") + + if binding_type == :list and not custom_widget? and + not MapSet.member?(@list_widgets, widget_type) do raise ArgumentError, "binding #{inspect(Map.get(binding, :id))} declares a list binding on #{inspect(widget_type)}, which does not expose collection semantics" end diff --git a/test/ash_ui/resource/locality_test.exs b/test/ash_ui/resource/locality_test.exs index 1ed1f728..601f46e5 100644 --- a/test/ash_ui/resource/locality_test.exs +++ b/test/ash_ui/resource/locality_test.exs @@ -1,6 +1,27 @@ defmodule AshUI.Resource.LocalityTest do use ExUnit.Case, async: true + # Asserts the binding-locality validator did NOT raise the + # "does not expose collection semantics" ArgumentError for the resource + # compiled inside `fun`. Any other exception (e.g. the unrelated + # domain-registration RuntimeError from the Ash compile-time pipeline) + # is treated as proof the validator passed; absence of any exception is + # also a pass. + defp assert_validator_accepts_list_binding(fun) do + fun.() + :ok + rescue + e in ArgumentError -> + if e.message =~ "does not expose collection semantics" do + flunk("validator rejected list binding it should accept: #{e.message}") + else + :ok + end + + _other -> + :ok + end + describe "screen-scoped binding exceptions" do test "rejects screen bindings that target element-owned props" do assert_raise ArgumentError, ~r/screen-scoped binding .* must target/, fn -> @@ -119,6 +140,50 @@ defmodule AshUI.Resource.LocalityTest do """) end end + + test "accepts list bindings on custom:* opaque widget types" do + # `custom:*` application-extension primitives are outside the canonical + # catalog; the validator can't know their collection semantics and + # should trust the authoring intent. + # + # Same testing strategy as the :artifact_row case above. + assert_validator_accepts_list_binding(fn -> + Code.compile_string(""" + defmodule ValidCustomWidgetListBinding do + use Ash.Resource, + domain: AshUI.Test.ResourceAuthorityDomain, + data_layer: Ash.DataLayer.Ets + + use AshUI.Resource.DSL.Element + + ets do + private?(true) + end + + attributes do + uuid_primary_key(:id) + end + + actions do + defaults([:read]) + end + + ui_element do + type "custom:thread_card" + props %{label: "Custom row template"} + end + + ui_bindings do + binding :thread_rows do + source %{resource: "Demo.Post", relationship: "comments", id: "post-1"} + target "items" + binding_type :list + end + end + end + """) + end) + end end describe "action signal ownership" do