fix(api): replace broken Ash.Expr.ref/2 filters with nested keyword filters

InTokenMap/ParentInTokenMap called Ash.Expr.ref/2 with reversed args
and, even fixed, dynamic relationship-path refs don't resolve through
filter hydration. Build filters as nested keyword lists instead (a
form FilterCheck.filter/3 accepts directly and Ash resolves natively
through relationships). Also fixes CreateParentInTokenMap's internal
parent lookup, which silently returned no rows without an actor due
to MapSystem's actor-scoped read preparation. Adds direct filter/3
tests against real queries/DB rows for both FilterCheck modules plus
positive/negative CreateParentInTokenMap coverage.

lib/wanderer_app/api/policies/map_scoped.ex

@@ -35,7 +35,19 @@ defmodule WandererApp.Api.Policies.MapScoped do
   end
 
   defmodule InTokenMap do
-    @moduledoc "Filters rows down to those belonging to the token's map."
+    @moduledoc """
+    Filters rows down to those belonging to the token's map.
+
+    `Ash.Expr.ref/2` takes `(relationship_path, name)`. A dynamically built
+    relationship-path ref (e.g. `Ash.Expr.ref([:system], :map_id)`) does not
+    resolve through filter hydration the same way a literal
+    `expr(system.map_id == ...)` does — it raises
+    `Ash.Error.Unknown: "Invalid reference ..."` (see task-1-report.md,
+    fix-round-1 notes). We therefore build the filter as a nested keyword
+    list, which `FilterCheck.filter/3` accepts directly
+    (`@callback filter(...) :: Keyword.t() | Ash.Expr.t()`) and which Ash
+    resolves through relationships without needing a dynamic `ref/2` call.
+    """
     use Ash.Policy.FilterCheck
     alias WandererApp.Api.ActorHelpers
 
@@ -47,19 +59,24 @@ defmodule WandererApp.Api.Policies.MapScoped do
       path = Keyword.fetch!(opts, :path)
 
       case ActorHelpers.get_map(%{actor: actor}) do
-        %{id: map_id} -> ref_eq(path, map_id)
+        %{id: map_id} -> build_filter(path, map_id)
         _ -> expr(false)
       end
     end
 
-    defp ref_eq(path, v) do
-      {rel, [attr]} = Enum.split(path, -1)
-      expr(^Ash.Expr.ref(attr, rel) == ^v)
+    defp build_filter(path, map_id) do
+      path
+      |> Enum.reverse()
+      |> Enum.reduce(map_id, fn segment, acc -> [{segment, acc}] end)
     end
   end
 
   defmodule ParentInTokenMap do
-    @moduledoc "Filters rows down to those whose parent (via relationship path) belongs to the token's map."
+    @moduledoc """
+    Filters rows down to those whose parent (via relationship path) belongs
+    to the token's map. See `InTokenMap` moduledoc for why the filter is
+    built as a nested keyword list rather than via a dynamic `ref/2` call.
+    """
     use Ash.Policy.FilterCheck
     alias WandererApp.Api.ActorHelpers
 
@@ -71,14 +88,16 @@ defmodule WandererApp.Api.Policies.MapScoped do
       path = Keyword.fetch!(opts, :path)
 
       case ActorHelpers.get_map(%{actor: actor}) do
-        %{id: map_id} ->
-          {rel, [attr]} = Enum.split(path, -1)
-          expr(^Ash.Expr.ref(attr, rel) == ^map_id)
-
-        _ ->
-          expr(false)
+        %{id: map_id} -> build_filter(path, map_id)
+        _ -> expr(false)
       end
     end
+
+    defp build_filter(path, map_id) do
+      path
+      |> Enum.reverse()
+      |> Enum.reduce(map_id, fn segment, acc -> [{segment, acc}] end)
+    end
   end
 
   defmodule WriteDirect do
@@ -126,6 +145,7 @@ defmodule WandererApp.Api.Policies.MapScoped do
            parent_id when not is_nil(parent_id) <- Ash.Changeset.get_attribute(cs, fk) do
         case Ash.read_one(
                Ash.Query.filter(parent, id == ^parent_id and map_id == ^map_id),
+               actor: actor,
                authorize?: false
              ) do
           {:ok, nil} -> false

test/wanderer_app/api/policies/map_scoped_test.exs

@@ -1,5 +1,6 @@
 defmodule WandererApp.Api.Policies.MapScopedTest do
   use WandererApp.DataCase, async: true
+  require Ash.Query
   alias WandererApp.Api.Policies.MapScoped
   alias WandererApp.Api.ActorWithMap
 
@@ -122,4 +123,148 @@ defmodule WandererApp.Api.Policies.MapScopedTest do
       refute MapScoped.CreateMapMatchesToken.match?(no_map_actor, %{changeset: cs}, [])
     end
   end
+
+  describe "InTokenMap filter/3 (direct path, exercised against a real query)" do
+    setup do
+      map = insert(:map, %{})
+      other_map = insert(:map, %{})
+      system_in_map = insert(:map_system, %{map_id: map.id})
+      system_in_other_map = insert(:map_system, %{map_id: other_map.id})
+      actor = ActorWithMap.new(%{id: "u"}, %{id: map.id})
+
+      %{
+        map: map,
+        actor: actor,
+        system_in_map: system_in_map,
+        system_in_other_map: system_in_other_map
+      }
+    end
+
+    test "returned filter admits only rows in the token map", %{
+      actor: actor,
+      system_in_map: system_in_map,
+      system_in_other_map: system_in_other_map
+    } do
+      filter = MapScoped.InTokenMap.filter(actor, %{}, path: [:map_id])
+
+      {:ok, results} =
+        WandererApp.Api.MapSystem
+        |> Ash.Query.filter(^filter)
+        |> Ash.read(actor: actor, authorize?: false)
+
+      ids = Enum.map(results, & &1.id)
+      assert system_in_map.id in ids
+      refute system_in_other_map.id in ids
+    end
+
+    test "returns expr(false) when actor has no token map" do
+      no_map_actor = ActorWithMap.new(%{id: "u"}, nil)
+      filter = MapScoped.InTokenMap.filter(no_map_actor, %{}, path: [:map_id])
+
+      {:ok, results} =
+        WandererApp.Api.MapSystem
+        |> Ash.Query.filter(^filter)
+        |> Ash.read(actor: no_map_actor, authorize?: false)
+
+      assert results == []
+    end
+  end
+
+  describe "ParentInTokenMap filter/3 (relationship path, exercised against a real query)" do
+    setup do
+      map = insert(:map, %{})
+      other_map = insert(:map, %{})
+      system_in_map = insert(:map_system, %{map_id: map.id})
+      system_in_other_map = insert(:map_system, %{map_id: other_map.id})
+
+      sig_in_map =
+        insert(:map_system_signature, %{system_id: system_in_map.id})
+
+      sig_in_other_map =
+        insert(:map_system_signature, %{system_id: system_in_other_map.id})
+
+      actor = ActorWithMap.new(%{id: "u"}, %{id: map.id})
+
+      %{actor: actor, sig_in_map: sig_in_map, sig_in_other_map: sig_in_other_map}
+    end
+
+    test "returned filter admits only rows whose parent belongs to the token map", %{
+      actor: actor,
+      sig_in_map: sig_in_map,
+      sig_in_other_map: sig_in_other_map
+    } do
+      filter = MapScoped.ParentInTokenMap.filter(actor, %{}, path: [:system, :map_id])
+
+      {:ok, results} =
+        WandererApp.Api.MapSystemSignature
+        |> Ash.Query.filter(^filter)
+        |> Ash.read(actor: actor, authorize?: false)
+
+      ids = Enum.map(results, & &1.id)
+      assert sig_in_map.id in ids
+      refute sig_in_other_map.id in ids
+    end
+
+    test "returns expr(false) when actor has no token map" do
+      no_map_actor = ActorWithMap.new(%{id: "u"}, nil)
+      filter = MapScoped.ParentInTokenMap.filter(no_map_actor, %{}, path: [:system, :map_id])
+
+      {:ok, results} =
+        WandererApp.Api.MapSystemSignature
+        |> Ash.Query.filter(^filter)
+        |> Ash.read(actor: no_map_actor, authorize?: false)
+
+      assert results == []
+    end
+  end
+
+  describe "CreateParentInTokenMap" do
+    setup do
+      map = insert(:map, %{})
+      other_map = insert(:map, %{})
+      system_in_map = insert(:map_system, %{map_id: map.id})
+      system_in_other_map = insert(:map_system, %{map_id: other_map.id})
+      actor = ActorWithMap.new(%{id: "u"}, %{id: map.id})
+
+      %{
+        actor: actor,
+        system_in_map: system_in_map,
+        system_in_other_map: system_in_other_map
+      }
+    end
+
+    test "authorizes when the parent (looked up by fk) belongs to the token map", %{
+      actor: actor,
+      system_in_map: system_in_map
+    } do
+      cs =
+        WandererApp.Api.MapSystemSignature
+        |> Ash.Changeset.new()
+        |> Ash.Changeset.force_change_attribute(:system_id, system_in_map.id)
+
+      assert MapScoped.CreateParentInTokenMap.match?(
+               actor,
+               %{changeset: cs},
+               parent_resource: WandererApp.Api.MapSystem,
+               fk: :system_id
+             )
+    end
+
+    test "denies when the parent belongs to a foreign map", %{
+      actor: actor,
+      system_in_other_map: system_in_other_map
+    } do
+      cs =
+        WandererApp.Api.MapSystemSignature
+        |> Ash.Changeset.new()
+        |> Ash.Changeset.force_change_attribute(:system_id, system_in_other_map.id)
+
+      refute MapScoped.CreateParentInTokenMap.match?(
+               actor,
+               %{changeset: cs},
+               parent_resource: WandererApp.Api.MapSystem,
+               fk: :system_id
+             )
+    end
+  end
 end