Merge pull request #637 from guarzo/fix/map-character-location-tracking

fix(test): repair 8 never-passing character location tracking tests

lib/wanderer_app/repositories/map_repo.ex

@@ -21,11 +21,33 @@ defmodule WandererApp.MapRepo do
       {:ok, map} ->
         map |> load_relationships(relationships)
 
-      _ ->
+      # Ash wraps NotFound inside Ash.Error.Invalid, so match the wrapped shape
+      # as well as a bare NotFound. Getting this wrong turns a genuine
+      # "map does not exist" into a generic error for every caller.
+      {:error, %Ash.Error.Query.NotFound{}} ->
         {:error, :not_found}
+
+      {:error, %Ash.Error.Invalid{errors: errors}} = error ->
+        if Enum.any?(errors, &match?(%Ash.Error.Query.NotFound{}, &1)) do
+          {:error, :not_found}
+        else
+          log_get_failure(map_id, error)
+          error
+        end
+
+      {:error, _reason} = error ->
+        log_get_failure(map_id, error)
+        error
     end
   end
 
+  # Previously every error was flattened into {:error, :not_found}, which masked
+  # infrastructure faults (e.g. DBConnection ownership errors) as "map does not
+  # exist" and made them very hard to diagnose.
+  defp log_get_failure(map_id, {:error, reason}) do
+    Logger.error("MapRepo.get failed for map #{inspect(map_id)}: #{inspect(reason)}")
+  end
+
   def get_by_slug_with_permissions(map_slug, current_user) do
     map_slug
     |> WandererApp.Api.Map.get_map_by_slug!()

test/integration/map/character_location_tracking_test.exs

@@ -19,6 +19,11 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
 
   use WandererApp.IntegrationCase, async: false
 
+  # These tests start real map servers. The MapPool GenServer loads map state
+  # from the database during init, so it needs shared sandbox mode -- see
+  # WandererApp.IntegrationCase.
+  @moduletag :shared_sandbox
+
   import Mox
 
   setup :verify_on_exit!
@@ -26,15 +31,29 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
   import WandererApp.MapTestHelpers
 
   alias WandererApp.Map.Server.CharactersImpl
-  alias WandererApp.Map.Server.SystemsImpl
 
   @test_character_eve_id 2_123_456_789
 
-  # EVE Online solar system IDs for testing
-  @system_jita 30_000_142
+  # EVE Online solar system IDs for testing.
+  #
+  # NOTE: Jita (30_000_142) is deliberately NOT used here. It is hardcoded as
+  # permanently un-addable in production:
+  #
+  #     # map_server_connections_impl.ex
+  #     @jita 30_000_142
+  #     @prohibited_systems [@jita]
+  #
+  # These tests previously used Jita as the "start system" and asserted it was
+  # added to the map, which asserts behaviour the application forbids by design
+  # and can never pass. Hek is a plain hi-sec system with no such restriction, so
+  # the intent of each test (a system the character moved away from gets added)
+  # is preserved while testing behaviour that is actually reachable.
+  # The prohibition itself is pinned by its own regression test below.
+  @system_hek 30_002_053
   @system_amarr 30_002_187
   @system_dodixie 30_002_659
   @system_rens 30_002_510
+  @system_jita_prohibited 30_000_142
 
   setup do
     # Setup system static info cache for test systems
@@ -65,6 +84,11 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
         slug: "test-char-tracking-#{:rand.uniform(1_000_000)}",
         owner_id: character.id,
         scope: :all,
+        # `scopes` (array) takes precedence over `scope` in
+        # CharactersImpl.get_effective_scopes/1 and defaults to [:wormholes].
+        # Without setting it, hi-sec movement is rejected as an invalid
+        # connection and no system is ever added.
+        scopes: [:hi, :low, :null, :pochven, :wormholes],
         only_tracked_characters: false
       })
 
@@ -93,7 +117,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
     @tag :integration
     test "character location update adds system to map", %{map: map, character: character} do
       # This test verifies the basic flow:
-      # 1. Character starts tracking on a map at Jita
+      # 1. Character starts tracking on a map at Hek
       # 2. Character moves to Amarr
       # 3. update_characters() is called
       # 4. Both systems are added to the map
@@ -102,23 +126,23 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
       ensure_map_started(map.id)
 
       # Setup: Add character to presence
-      add_character_to_map_presence(map.id, character.id)
+      track_character_on_map(map.id, character.id)
 
-      # Setup: Character starts at Jita
-      set_character_location(character.id, @system_jita)
+      # Setup: Character starts at Hek
+      set_character_location(character.id, @system_hek)
 
       # Setup: Set start_solar_system_id (this happens when tracking starts)
       # Note: The start system is NOT added until the character moves
       WandererApp.Cache.insert(
         "map:#{map.id}:character:#{character.id}:start_solar_system_id",
-        @system_jita
+        @system_hek
       )
 
       # Execute: First update - start system is intentionally NOT added yet
       CharactersImpl.update_characters(map.id)
 
-      # Verify: Jita should NOT be on map yet (design: start position not added)
-      refute system_on_map?(map.id, @system_jita),
+      # Verify: Hek should NOT be on map yet (design: start position not added)
+      refute system_on_map?(map.id, @system_hek),
              "Start system should not be added until character moves"
 
       # Character moves to Amarr
@@ -128,8 +152,8 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
       CharactersImpl.update_characters(map.id)
 
       # Verify: Both systems should now be on map
-      assert wait_for_system_on_map(map.id, @system_jita),
-             "Jita should be added after character moves"
+      assert wait_for_system_on_map(map.id, @system_hek),
+             "Hek should be added after character moves"
 
       assert wait_for_system_on_map(map.id, @system_amarr),
              "Amarr should be added as the new location"
@@ -148,20 +172,20 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
       ensure_map_started(map.id)
 
       # Setup: Add character to presence
-      add_character_to_map_presence(map.id, character.id)
+      track_character_on_map(map.id, character.id)
 
-      # Setup: Character starts at Jita
-      set_character_location(character.id, @system_jita)
+      # Setup: Character starts at Hek
+      set_character_location(character.id, @system_hek)
 
       WandererApp.Cache.insert(
         "map:#{map.id}:character:#{character.id}:start_solar_system_id",
-        @system_jita
+        @system_hek
       )
 
       # First update - start system is intentionally NOT added yet
       CharactersImpl.update_characters(map.id)
 
-      refute system_on_map?(map.id, @system_jita),
+      refute system_on_map?(map.id, @system_hek),
              "Start system should not be added until character moves"
 
       # Character moves to Amarr
@@ -171,8 +195,8 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
       CharactersImpl.update_characters(map.id)
 
       # Verify: Both systems should be on map after character moves
-      assert wait_for_system_on_map(map.id, @system_jita),
-             "Jita should be added after character moves"
+      assert wait_for_system_on_map(map.id, @system_hek),
+             "Hek should be added after character moves"
 
       assert wait_for_system_on_map(map.id, @system_amarr),
              "Amarr should be added as the new location"
@@ -188,31 +212,31 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
       # Note: Start system is NOT added until character moves (design decision)
 
       ensure_map_started(map.id)
-      add_character_to_map_presence(map.id, character.id)
+      track_character_on_map(map.id, character.id)
 
-      # Character starts at Jita
-      set_character_location(character.id, @system_jita)
+      # Character starts at Hek
+      set_character_location(character.id, @system_hek)
 
       WandererApp.Cache.insert(
         "map:#{map.id}:character:#{character.id}:start_solar_system_id",
-        @system_jita
+        @system_hek
       )
 
       # First update - start system is intentionally NOT added yet
       CharactersImpl.update_characters(map.id)
 
-      refute system_on_map?(map.id, @system_jita),
+      refute system_on_map?(map.id, @system_hek),
              "Start system should not be added until character moves"
 
       # Rapid jump to Amarr (intermediate system)
       set_character_location(character.id, @system_amarr)
 
-      # Second update - should add both Jita (start) and Amarr (current)
+      # Second update - should add both Hek (start) and Amarr (current)
       CharactersImpl.update_characters(map.id)
 
-      # Verify both Jita and Amarr are now on map
-      assert wait_for_system_on_map(map.id, @system_jita),
-             "Jita (start) should be on map after movement"
+      # Verify both Hek and Amarr are now on map
+      assert wait_for_system_on_map(map.id, @system_hek),
+             "Hek (start) should be on map after movement"
 
       assert wait_for_system_on_map(map.id, @system_amarr), "Amarr should be on map"
 
@@ -223,7 +247,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
       CharactersImpl.update_characters(map.id)
 
       # Verify: All three systems should be on map
-      assert wait_for_system_on_map(map.id, @system_jita), "Jita (start) should still be on map"
+      assert wait_for_system_on_map(map.id, @system_hek), "Hek (start) should still be on map"
 
       assert wait_for_system_on_map(map.id, @system_amarr),
              "Amarr (intermediate) should still be on map - this is the critical test"
@@ -240,21 +264,21 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
       # don't cause intermediate systems to be lost due to cache races.
 
       ensure_map_started(map.id)
-      add_character_to_map_presence(map.id, character.id)
+      track_character_on_map(map.id, character.id)
 
-      # Start at Jita
-      set_character_location(character.id, @system_jita)
+      # Start at Hek
+      set_character_location(character.id, @system_hek)
 
       WandererApp.Cache.insert(
         "map:#{map.id}:character:#{character.id}:start_solar_system_id",
-        @system_jita
+        @system_hek
       )
 
       CharactersImpl.update_characters(map.id)
 
       # Simulate rapid updates happening faster than update_characters cycle (1 second)
       # Jump through 4 systems in quick succession
-      systems = [@system_amarr, @system_dodixie, @system_rens, @system_jita]
+      systems = [@system_amarr, @system_dodixie, @system_rens, @system_hek]
 
       for system <- systems do
         set_character_location(character.id, system)
@@ -266,7 +290,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
 
       # Verify: All systems should eventually be on the map
       # Even if some updates happened concurrently
-      for system <- [@system_jita | systems] do
+      for system <- [@system_hek | systems] do
         assert wait_for_system_on_map(map.id, system),
                "System #{system} should be on map despite rapid movements"
       end
@@ -283,15 +307,15 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
       # start_solar_system_id should not be lost after first use
 
       ensure_map_started(map.id)
-      add_character_to_map_presence(map.id, character.id)
+      track_character_on_map(map.id, character.id)
 
-      # Set character at Jita
-      set_character_location(character.id, @system_jita)
+      # Set character at Hek
+      set_character_location(character.id, @system_hek)
 
       # Set start_solar_system_id
       WandererApp.Cache.insert(
         "map:#{map.id}:character:#{character.id}:start_solar_system_id",
-        @system_jita
+        @system_hek
       )
 
       # First update
@@ -301,7 +325,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
       {:ok, start_system} =
         WandererApp.Cache.lookup("map:#{map.id}:character:#{character.id}:start_solar_system_id")
 
-      assert start_system == @system_jita,
+      assert start_system == @system_hek,
              "start_solar_system_id should persist after first update (not be taken/removed)"
 
       # Character moves to Amarr
@@ -311,7 +335,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
       CharactersImpl.update_characters(map.id)
 
       # Verify both systems are on map
-      assert wait_for_system_on_map(map.id, @system_jita)
+      assert wait_for_system_on_map(map.id, @system_hek)
       assert wait_for_system_on_map(map.id, @system_amarr)
     end
 
@@ -325,22 +349,22 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
       # Design: Start system is NOT added until character moves
 
       ensure_map_started(map.id)
-      add_character_to_map_presence(map.id, character.id)
+      track_character_on_map(map.id, character.id)
 
-      # Character is at Jita, no previous location
-      set_character_location(character.id, @system_jita)
+      # Character is at Hek, no previous location
+      set_character_location(character.id, @system_hek)
 
       # Set start_solar_system_id
       WandererApp.Cache.insert(
         "map:#{map.id}:character:#{character.id}:start_solar_system_id",
-        @system_jita
+        @system_hek
       )
 
       # First update - character still at start position
       CharactersImpl.update_characters(map.id)
 
-      # Verify Jita is NOT added yet (design: start position not added until movement)
-      refute system_on_map?(map.id, @system_jita),
+      # Verify Hek is NOT added yet (design: start position not added until movement)
+      refute system_on_map?(map.id, @system_hek),
              "Start system should not be added until character moves"
 
       # Character moves to Amarr
@@ -350,8 +374,8 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
       CharactersImpl.update_characters(map.id)
 
       # Verify both systems are added after movement
-      assert wait_for_system_on_map(map.id, @system_jita),
-             "Jita should be added after character moves away"
+      assert wait_for_system_on_map(map.id, @system_hek),
+             "Hek should be added after character moves away"
 
       assert wait_for_system_on_map(map.id, @system_amarr),
              "Amarr should be added as the new location"
@@ -366,7 +390,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
       # (Retry logic not yet implemented)
 
       ensure_map_started(map.id)
-      add_character_to_map_presence(map.id, character.id)
+      track_character_on_map(map.id, character.id)
 
       test_pid = self()
 
@@ -380,12 +404,12 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
         nil
       )
 
-      # Set character at Jita and set start location
-      set_character_location(character.id, @system_jita)
+      # Set character at Hek and set start location
+      set_character_location(character.id, @system_hek)
 
       WandererApp.Cache.insert(
         "map:#{map.id}:character:#{character.id}:start_solar_system_id",
-        @system_jita
+        @system_hek
       )
 
       # Trigger update which may encounter database issues
@@ -411,14 +435,14 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
       # and logged without crashing the entire update_characters cycle
 
       ensure_map_started(map.id)
-      add_character_to_map_presence(map.id, character.id)
+      track_character_on_map(map.id, character.id)
 
       # Set up character location
-      set_character_location(character.id, @system_jita)
+      set_character_location(character.id, @system_hek)
 
       WandererApp.Cache.insert(
         "map:#{map.id}:character:#{character.id}:start_solar_system_id",
-        @system_jita
+        @system_hek
       )
 
       # Run update_characters - should complete even if individual character updates fail
@@ -478,14 +502,14 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
       # Note: Recovery ETS table not yet implemented
 
       ensure_map_started(map.id)
-      add_character_to_map_presence(map.id, character.id)
+      track_character_on_map(map.id, character.id)
 
       # Set up character with location
-      set_character_location(character.id, @system_jita)
+      set_character_location(character.id, @system_hek)
 
       WandererApp.Cache.insert(
         "map:#{map.id}:character:#{character.id}:start_solar_system_id",
-        @system_jita
+        @system_hek
       )
 
       # Run multiple update cycles to verify stability
@@ -528,10 +552,10 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
             })
 
           # Add character to presence and set location
-          add_character_to_map_presence(map.id, character.id)
+          track_character_on_map(map.id, character.id)
 
           solar_system_id =
-            Enum.at([@system_jita, @system_amarr, @system_dodixie, @system_rens], rem(i, 4))
+            Enum.at([@system_hek, @system_amarr, @system_dodixie, @system_rens], rem(i, 4))
 
           set_character_location(character.id, solar_system_id)
 
@@ -563,7 +587,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
       # emit proper telemetry events for monitoring
 
       ensure_map_started(map.id)
-      add_character_to_map_presence(map.id, character.id)
+      track_character_on_map(map.id, character.id)
 
       test_pid = self()
 
@@ -582,7 +606,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
       )
 
       # Set up character location
-      set_character_location(character.id, @system_jita)
+      set_character_location(character.id, @system_hek)
 
       # Trigger update_characters
       CharactersImpl.update_characters(map.id)
@@ -615,14 +639,14 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
       # 3. Cachex.get(:character_state_cache, character_id) - character state
 
       ensure_map_started(map.id)
-      add_character_to_map_presence(map.id, character.id)
+      track_character_on_map(map.id, character.id)
 
       # Set location in character cache
-      set_character_location(character.id, @system_jita)
+      set_character_location(character.id, @system_hek)
 
       WandererApp.Cache.insert(
         "map:#{map.id}:character:#{character.id}:start_solar_system_id",
-        @system_jita
+        @system_hek
       )
 
       CharactersImpl.update_characters(map.id)
@@ -631,7 +655,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
       {:ok, map_cached_location} =
         WandererApp.Cache.lookup("map:#{map.id}:character:#{character.id}:solar_system_id")
 
-      assert map_cached_location == @system_jita,
+      assert map_cached_location == @system_hek,
              "Map-specific cache should match character cache"
 
       # Move character
@@ -685,7 +709,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
         %{
           character_id: character.id,
           map_id: map.id,
-          from_system: @system_jita,
+          from_system: @system_hek,
           to_system: @system_amarr
         }
       )
@@ -702,4 +726,43 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
       :telemetry.detach("test-character-location-events")
     end
   end
+
+  describe "Prohibited systems" do
+    @tag :integration
+    test "Jita is never added to a map, even when a character moves through it",
+         %{map: map, character: character} do
+      # Pins the reason every other test in this module uses Hek rather than
+      # Jita as its start system. Jita is hardcoded as permanently un-addable
+      # (`@prohibited_systems [@jita]` in map_server_connections_impl.ex).
+      #
+      # If this test starts failing, the prohibition was changed or removed --
+      # revisit the constants at the top of this file rather than deleting this.
+      ensure_map_started(map.id)
+      track_character_on_map(map.id, character.id)
+
+      set_character_location(character.id, @system_jita_prohibited)
+
+      WandererApp.Cache.insert(
+        "map:#{map.id}:character:#{character.id}:start_solar_system_id",
+        @system_jita_prohibited
+      )
+
+      CharactersImpl.update_characters(map.id)
+
+      # Move away from Jita. For any non-prohibited system this is exactly the
+      # sequence that causes the start system to be added to the map.
+      set_character_location(character.id, @system_amarr)
+      CharactersImpl.update_characters(map.id)
+
+      # The prohibition is evaluated per-connection, not per-system: Jita being
+      # prohibited invalidates the whole Jita->Amarr movement, so neither
+      # endpoint is added. Assert both, so the test pins the actual semantics
+      # rather than a convenient half of them.
+      refute system_on_map?(map.id, @system_jita_prohibited),
+             "Jita is in @prohibited_systems and must never be added to a map"
+
+      refute system_on_map?(map.id, @system_amarr),
+             "A connection touching a prohibited system is rejected entirely"
+    end
+  end
 end

test/support/integration_case.ex

@@ -2,17 +2,19 @@ defmodule WandererApp.IntegrationCase do
   @moduledoc """
   This module defines the test case for integration tests.
 
-  Integration tests use shared sandbox mode (`shared: true`) when running async
-  to avoid timing issues with dynamically spawned processes like MapPool GenServers
-  that need database access immediately upon spawn.
+  Integration tests default to a private sandbox owner. Suites that start real
+  map servers must opt into shared sandbox mode:
 
-  For async integration tests, shared mode allows:
-  - MapPool GenServers to access the database without explicit allowance
-  - Tests to run in parallel without complex permission granting
-  - Reliable test execution without race conditions
+      use WandererApp.IntegrationCase, async: false
+      @moduletag :shared_sandbox
 
-  For synchronous integration tests, shared mode is disabled (shared: false)
-  for better isolation.
+  Shared mode is required when a dynamically spawned process queries the
+  database immediately upon spawn (MapPool GenServers load map state during
+  `init`), because there is no window in which the test process can allow it
+  onto the connection first.
+
+  Shared mode is node-global, so it is opt-in and reverted on exit. It is only
+  valid with `async: false`.
 
   Use this case for:
   - API controller integration tests that spawn map servers
@@ -62,15 +64,18 @@ defmodule WandererApp.IntegrationCase do
   end
 
   @doc """
-  Sets up the sandbox with shared mode for async integration tests.
+  Sets up the test sandbox, per the `:shared_sandbox` moduletag.
 
-  For async tests (async: true):
-  - Uses shared: true to allow dynamically spawned processes database access
-  - Trades some isolation for reliability and simplicity
+  With `@moduletag :shared_sandbox` (only valid with `async: false`):
+  - Uses shared: true so dynamically spawned processes (e.g. MapPool
+    GenServers that query the DB during `init`) get database access
+  - Trades some isolation for reliability with background processes
 
-  For sync tests (async: false):
-  - Uses shared: false for better isolation
+  Without the tag (the default):
+  - Starts a dedicated, private sandbox owner (shared: false)
   - Child processes require explicit allowance
+
+  Raises `ArgumentError` if `:shared_sandbox` is set on an `async: true` suite.
   """
   def setup_sandbox(tags) do
     # Ensure the repo is started before setting up sandbox
@@ -78,20 +83,39 @@ defmodule WandererApp.IntegrationCase do
       {:ok, _} = WandererApp.Repo.start_link()
     end
 
-    # For integration tests:
-    # - Use shared: true for async tests to avoid MapPool timing issues
-    # - Use shared: false for sync tests for better isolation
-    shared_mode = tags[:async] == true
+    # Shared mode is opt-in per suite via `@moduletag :shared_sandbox`.
+    #
+    # Suites that start real map servers need it: MapPool GenServers are spawned
+    # dynamically and load map state from the DB *during init*, so there is no
+    # point at which the test process can allow them onto the connection first
+    # (polling loses the race). Shared mode is the only mechanism that covers a
+    # process that queries immediately upon spawn.
+    #
+    # It is opt-in rather than global because Sandbox shared mode applies to the
+    # whole node: enabling it for every sync integration suite regresses suites
+    # that rely on owner-private connections. Ecto only permits shared mode when
+    # the test is not async, so the tag is rejected on async suites.
+    shared_mode = tags[:shared_sandbox] == true
+
+    if shared_mode and tags[:async] == true do
+      raise ArgumentError,
+            "#{inspect(tags[:module])} sets @moduletag :shared_sandbox but is `async: true`. " <>
+              "Ecto sandbox shared mode is node-global and is only safe with `async: false`."
+    end
 
     # Set up sandbox mode based on test type
     pid =
       if shared_mode do
-        # For async tests with shared mode:
-        # Checkout the sandbox connection instead of starting an owner
-        # This allows multiple async tests to use the same connection pool
         :ok = Ecto.Adapters.SQL.Sandbox.checkout(WandererApp.Repo)
-        # Put the connection in shared mode
         Ecto.Adapters.SQL.Sandbox.mode(WandererApp.Repo, {:shared, self()})
+
+        # Shared mode is node-global, so it MUST be reverted to :manual when the
+        # test ends. Leaving it set leaks into every later suite on the node and
+        # is why an earlier global-flip attempt broke unrelated controller tests.
+        on_exit(fn ->
+          Ecto.Adapters.SQL.Sandbox.mode(WandererApp.Repo, :manual)
+        end)
+
         self()
       else
         # For sync tests, start a dedicated owner
@@ -117,10 +141,15 @@ defmodule WandererApp.IntegrationCase do
   Allows a process to access the database by granting it sandbox access.
   This is necessary for background processes that need database access in non-shared mode.
   """
-  def allow_database_access(pid) when is_pid(pid) do
-    owner_pid = Process.get(:sandbox_owner_pid)
+  def allow_database_access(pid, owner_pid \\ nil) when is_pid(pid) do
+    owner_pid = owner_pid || Process.get(:sandbox_owner_pid)
 
     if owner_pid do
+      # Returns `:ok | {:already, :owner | :allowed}` on every poll tick;
+      # re-allowing an already-allowed process is a normal, non-error case, so
+      # both are treated as success here. It raises only for infrastructure
+      # faults (repo not started, or `pid`/`owner_pid` not resolving to a live
+      # process) -- those should surface rather than be swallowed.
       Ecto.Adapters.SQL.Sandbox.allow(WandererApp.Repo, owner_pid, pid)
     end
   end
@@ -192,6 +221,23 @@ defmodule WandererApp.IntegrationCase do
             |> Enum.filter(&is_pid/1)
             |> Enum.filter(&Process.alive?/1)
             |> Enum.each(fn child_pid ->
+              # Grant BOTH mock ownership and Ecto sandbox access.
+              #
+              # MapPool GenServers are spawned dynamically *after* the one-shot
+              # grant_supervision_tree_access/2 call above has already run, so
+              # they are never on the sandbox connection. Their map-state
+              # loaders (Task.async in Map.Server.Impl.do_init_state/1) then die
+              # with DBConnection.OwnershipError, the error is reported as
+              # "map not loaded", and the test surfaces only a misleading
+              # "Timeout waiting for map ... Check Map.Manager is running".
+              #
+              # Sandbox.allow/3 also propagates to the pool's Task.async
+              # children via $callers, which is what the loaders rely on.
+              #
+              # owner_pid is passed explicitly: this runs inside the spawned
+              # monitor process, where the :sandbox_owner_pid process dict entry
+              # set by setup_sandbox/1 is not visible.
+              allow_database_access(child_pid, owner_pid)
               WandererApp.Test.MockOwnership.allow_mocks_for_process(child_pid, owner_pid)
             end)
 

test/support/map_test_helpers.ex

@@ -322,6 +322,29 @@ defmodule WandererApp.MapTestHelpers do
         wandering: [],
         triglavian_invasion_status: nil,
         sun_type_id: 45041
+      },
+      # Hek. Used as the default "start system" in location-tracking tests
+      # because, unlike Jita, it is not in @prohibited_systems and so can
+      # actually be added to a map.
+      30_002_053 => %{
+        solar_system_id: 30_002_053,
+        region_id: 10_000_042,
+        constellation_id: 20_000_302,
+        solar_system_name: "Hek",
+        solar_system_name_lc: "hek",
+        constellation_name: "Hedgehog",
+        region_name: "Metropolis",
+        system_class: 0,
+        security: "0.5",
+        type_description: "High Security",
+        class_title: "High Sec",
+        is_shattered: false,
+        effect_name: nil,
+        effect_power: nil,
+        statics: [],
+        wandering: [],
+        triglavian_invasion_status: nil,
+        sun_type_id: 45041
       }
     }
   end
@@ -333,7 +356,7 @@ defmodule WandererApp.MapTestHelpers do
   ## Parameters
   - character_id: The character ID to update
   - solar_system_id: The solar system ID where the character is located
-  - opts: Optional parameters (structure_id, station_id, ship)
+  - opts: Optional parameters (structure_id, station_id, ship, online)
 
   ## Examples
       iex> set_character_location(character.id, 30_000_142, ship: 670)
@@ -349,12 +372,18 @@ defmodule WandererApp.MapTestHelpers do
     {:ok, existing_character} = WandererApp.Character.get_character(character_id)
 
     # Update character cache (mimics Character.update_character/2)
+    # `online: true` is required, not cosmetic. When a character is offline the
+    # `{:character_online, ...}` handler in CharactersImpl DELETES the
+    # `map:<id>:character:<id>:solar_system_id` cache key that
+    # `check_location_update` just wrote, so no location change is ever observed
+    # and no system is added. Characters reporting a location are online.
     character_data =
       Map.merge(existing_character, %{
         solar_system_id: solar_system_id,
         structure_id: structure_id,
         station_id: station_id,
         ship: ship,
+        online: Keyword.get(opts, :online, true),
         updated_at: DateTime.utc_now()
       })
 
@@ -379,6 +408,45 @@ defmodule WandererApp.MapTestHelpers do
     WandererApp.Cache.insert("map_#{map_id}:presence_character_ids", updated_chars)
   end
 
+  @doc """
+  Makes a character actually tracked for `update_characters/1`.
+
+  Presence alone is NOT enough. `WandererApp.Map.get_tracked_character_ids/1`
+  requires both:
+
+    * the character to be registered on the map (`map.characters`), and
+    * a `character:<id>:map:<id>:tracking_start_time` cache key
+
+  Production sets these via `CharactersImpl.track_character/2`, which needs real
+  `MapCharacterSettings` rows and ESI access tokens. Tests short-circuit to the
+  same end state, mirroring `Character.TrackingUtils.track_character/3`.
+
+  Without this, `update_characters/1` iterates an empty list and silently does
+  nothing, which reads as "the feature is broken" rather than "the character was
+  never tracked".
+  """
+  def track_character_on_map(map_id, character_id) do
+    add_character_to_map_presence(map_id, character_id)
+
+    {:ok, character} = WandererApp.Character.get_character(character_id)
+    :ok = WandererApp.Map.add_character(map_id, character)
+
+    WandererApp.Cache.insert(
+      "character:#{character_id}:map:#{map_id}:tracking_start_time",
+      DateTime.utc_now()
+    )
+
+    # Production clears stale per-map location caches when tracking starts
+    # (TrackingUtils.track_character/4). Mirror it, so a character tracked after
+    # a previous location was cached does not appear to have "already been" at
+    # that system.
+    WandererApp.Cache.delete("map:#{map_id}:character:#{character_id}:solar_system_id")
+    WandererApp.Cache.delete("map:#{map_id}:character:#{character_id}:station_id")
+    WandererApp.Cache.delete("map:#{map_id}:character:#{character_id}:structure_id")
+
+    :ok
+  end
+
   @doc """
   Helper to get all systems currently on the map.
   Uses :map_cache instead of :map_state_cache because add_system/2 updates :map_cache.