Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .dialyzer_ignore.exs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
# Mix functions are only available during Mix compilation context
{"lib/mix/tasks/phoenix_kit.install.ex", :unknown_function},
{"lib/mix/tasks/phoenix_kit.update.ex", :unknown_function},
# Conditional compilation pattern match in update.ex (Code.ensure_loaded?)
{"lib/mix/tasks/phoenix_kit.update.ex", :pattern_match, 1},
{"lib/mix/tasks/phoenix_kit.modernize_layouts.ex", :unknown_function},
{"lib/phoenix_kit/install/migration_strategy.ex", :unknown_function},
{"lib/phoenix_kit/install/repo_detection.ex", :unknown_function},
Expand Down
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
## [Unreleased]

### Fixed
- **CRITICAL: Supervisor Ordering** - Fixed startup crashes caused by incorrect supervisor order in application.ex
- PhoenixKit.Supervisor and Oban now correctly start AFTER Repo instead of before
- Added explicit positioning using `after: [repo]` in Igniter installation logic
- Replaced text-based supervisor injection with proper Igniter.Project.Application API
- Added automatic fix in `mix phoenix_kit.update` to correct existing installations
- Prevents crashes: "Repo not ready" errors when loading Settings cache or Oban jobs
- Ensures correct order: Repo → PhoenixKit.Supervisor → Oban → Endpoint
## 1.6.8 - 2025-11-23
- Fix Oban configuration detection and automatic restart
- Improve status check messages to include Oban configuration
Expand Down
271 changes: 271 additions & 0 deletions lib/mix/tasks/phoenix_kit.update.ex
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ if Code.ensure_loaded?(Igniter.Mix.Task) do
BasicConfiguration,
Common,
CssIntegration,
IgniterHelpers,
ObanConfig,
RateLimiterConfig
}
Expand Down Expand Up @@ -338,6 +339,10 @@ if Code.ensure_loaded?(Igniter.Mix.Task) do
# Ensure Oban configuration exists
igniter = validate_and_add_oban_config(igniter)

# CRITICAL FIX: Ensure correct supervisor ordering in application.ex
# This must run AFTER add_oban_supervisor to fix installations with wrong order
igniter = fix_supervisor_ordering(igniter)

# Check if this is the first pass (config missing) or second pass (config exists)
config_status = Process.get(:phoenix_kit_config_status, :ok)

Expand Down Expand Up @@ -915,6 +920,272 @@ if Code.ensure_loaded?(Igniter.Mix.Task) do
end

# Validate and add Oban configuration if missing
# Fix supervisor ordering in application.ex to prevent startup crashes
# Ensures correct order: Repo → PhoenixKit.Supervisor → Oban → Endpoint
defp fix_supervisor_ordering(igniter) do
app_name = IgniterHelpers.get_parent_app_name(igniter)
app_file = "lib/#{app_name}/application.ex"

if File.exists?(app_file) do
content = File.read!(app_file)

# Check current supervisor ordering
case check_supervisor_order(content, app_name) do
:correct ->
# Order is already correct, no changes needed
igniter

{:needs_fix, reason} ->
# Order is incorrect, attempt to fix using Igniter API
igniter
|> fix_application_supervisor_order(app_name, reason)
|> add_supervisor_ordering_fixed_notice(reason)

:cannot_determine ->
# Cannot determine order (unusual setup), skip silently
igniter
end
else
# No application.ex found (unusual), skip
igniter
end
rescue
e ->
# If any error occurs, log warning but continue
Mix.shell().info("⚠️ Could not check supervisor ordering: #{inspect(e)}")
igniter
end

# Check the ordering of supervisors in application.ex
# Returns :correct, {:needs_fix, reason}, or :cannot_determine
defp check_supervisor_order(content, app_name) do
lines = String.split(content, "\n")

# Find line numbers for each supervisor
repo_line = find_supervisor_line(lines, ~r/#{app_name}\.Repo[,\s]/)
phoenix_kit_line = find_supervisor_line(lines, ~r/PhoenixKit\.Supervisor[,\s]/)

oban_line =
find_supervisor_line(lines, ~r/\{Oban,|Application\.get_env\(:#{app_name}, Oban\)/)

validate_supervisor_positions(repo_line, phoenix_kit_line, oban_line)
end

# Validate supervisor positions and return check result
defp validate_supervisor_positions(nil, nil, nil), do: :cannot_determine
defp validate_supervisor_positions(nil, _, _), do: :cannot_determine
defp validate_supervisor_positions(repo, nil, nil) when is_integer(repo), do: :correct

defp validate_supervisor_positions(repo, pk, nil) when is_integer(repo) and is_integer(pk) do
if repo < pk, do: :correct, else: {:needs_fix, "PhoenixKit.Supervisor before Repo"}
end

defp validate_supervisor_positions(repo, pk, oban)
when is_integer(repo) and is_integer(pk) and is_integer(oban) do
check_three_supervisor_order(repo, pk, oban)
end

defp validate_supervisor_positions(_, _, _), do: :cannot_determine

# Check ordering when all three supervisors exist
defp check_three_supervisor_order(repo, pk, oban) do
cond do
pk < repo and oban < repo -> {:needs_fix, "both PhoenixKit and Oban before Repo"}
pk < repo -> {:needs_fix, "PhoenixKit.Supervisor before Repo"}
oban < repo -> {:needs_fix, "Oban before Repo"}
oban < pk -> {:needs_fix, "Oban before PhoenixKit.Supervisor"}
true -> :correct
end
end

# Find the line number where a supervisor is defined
defp find_supervisor_line(lines, pattern) do
lines
|> Enum.with_index(1)
|> Enum.find(fn {line, _index} ->
trimmed = String.trim(line)
# Not a comment and matches pattern
!String.starts_with?(trimmed, "#") and Regex.match?(pattern, line)
end)
|> case do
{_line, index} -> index
nil -> nil
end
end

# Fix the supervisor ordering using manual reordering
# Note: We can't use Igniter.Project.Application.add_new_child to reorder existing children,
# so we need to manually reorder the children list
defp fix_application_supervisor_order(igniter, app_name, _reason) do
app_file = "lib/#{app_name}/application.ex"

Igniter.update_file(igniter, app_file, fn source ->
content = Rewrite.Source.get(source, :content)
fixed_content = reorder_supervisors(content, app_name)
Rewrite.Source.update(source, :content, fixed_content)
end)
end

# Reorder supervisors in application.ex to correct order
defp reorder_supervisors(content, app_name) do
lines = String.split(content, "\n")

# Extract supervisor lines
{repo_line, repo_index} = extract_supervisor(lines, ~r/#{app_name}\.Repo[,\s]/)
{pk_line, pk_index} = extract_supervisor(lines, ~r/PhoenixKit\.Supervisor[,\s]/)

{oban_line, oban_index} =
extract_supervisor(lines, ~r/\{Oban,|Application\.get_env\(:#{app_name}, Oban\)/)

# Determine children list boundaries
children_start = find_children_list_start(lines)
children_end = find_children_list_end(lines, children_start)

if is_integer(children_start) and is_integer(children_end) do
# Build new children list with correct order
supervisors = %{
repo: {repo_line, repo_index},
phoenix_kit: {pk_line, pk_index},
oban: {oban_line, oban_index}
}

new_lines =
rebuild_children_list(lines, children_start, children_end, supervisors)

Enum.join(new_lines, "\n")
else
# Cannot find children list boundaries, return unchanged
content
end
end

# Extract supervisor line and its index
defp extract_supervisor(lines, pattern) do
case Enum.with_index(lines, 1) do
indexed_lines ->
case Enum.find(indexed_lines, fn {line, _index} ->
trimmed = String.trim(line)
!String.starts_with?(trimmed, "#") and Regex.match?(pattern, line)
end) do
{line, index} -> {line, index}
nil -> {nil, nil}
end
end
end

# Find the start of children list
defp find_children_list_start(lines) do
Enum.find_index(lines, fn line ->
String.contains?(line, "children = [")
end)
end

# Find the end of children list (closing bracket)
defp find_children_list_end(lines, start_index) do
lines
|> Enum.drop(start_index + 1)
|> Enum.with_index(start_index + 1)
|> Enum.find(fn {line, _index} ->
trimmed = String.trim(line)
trimmed == "]"
end)
|> case do
{_line, index} -> index
nil -> nil
end
end

# Rebuild children list with correct supervisor ordering
defp rebuild_children_list(lines, children_start, children_end, supervisors) do
%{
repo: {repo_line, repo_index},
phoenix_kit: {pk_line, pk_index},
oban: {oban_line, oban_index}
} = supervisors

# Lines before children list
before_children = Enum.take(lines, children_start + 1)

# Lines after children list
after_children = Enum.drop(lines, children_end)

# Get all children between start and end
children_lines =
lines
|> Enum.drop(children_start + 1)
|> Enum.take(children_end - children_start - 1)

# Remove repo, phoenix_kit, and oban lines from children
filtered_children =
children_lines
|> Enum.with_index(children_start + 2)
|> Enum.reject(fn {_line, index} ->
index in [repo_index, pk_index, oban_index]
end)
|> Enum.map(fn {line, _index} -> line end)

# Build new ordered children list
ordered_children =
build_ordered_supervisor_list(repo_line, pk_line, oban_line, filtered_children)

# Reconstruct file
before_children ++ ordered_children ++ after_children
end

# Build ordered list of supervisors with correct positioning
defp build_ordered_supervisor_list(repo_line, pk_line, oban_line, filtered_children) do
# Add Repo first (if exists)
ordered = if repo_line, do: [repo_line], else: []

# Split remaining children at Endpoint
{before_endpoint, from_endpoint} = split_at_endpoint(filtered_children)

# Add PhoenixKit after Repo, before Endpoint
ordered = ordered ++ before_endpoint
ordered = if pk_line, do: ordered ++ [pk_line], else: ordered

# Add Oban after PhoenixKit
ordered = if oban_line, do: ordered ++ [oban_line], else: ordered

# Add remaining children (Endpoint and after)
ordered ++ from_endpoint
end

# Split children at Endpoint line
defp split_at_endpoint(children) do
endpoint_index =
Enum.find_index(children, fn line ->
String.contains?(line, "Endpoint") and !String.contains?(line, "#")
end)

case endpoint_index do
nil -> {children, []}
index -> Enum.split(children, index)
end
end

# Add notice about supervisor ordering being fixed
defp add_supervisor_ordering_fixed_notice(igniter, reason) do
notice = """
⚠️ CRITICAL FIX APPLIED: Corrected supervisor ordering in application.ex

Issue detected: #{reason}

Fixed to correct order:
1. YourApp.Repo (database connection - must be first)
2. PhoenixKit.Supervisor (uses Repo for Settings cache)
3. {Oban, ...} (uses Repo for job persistence)
4. Other supervisors...

This fixes startup crashes where PhoenixKit or Oban tried to access
the database before Repo was ready.

IMPORTANT: Restart your server for changes to take effect.
"""

Igniter.add_notice(igniter, String.trim(notice))
end

defp validate_and_add_oban_config(igniter) do
config_exists = ObanConfig.oban_config_exists?(igniter)
supervisor_exists = ObanConfig.oban_supervisor_exists?(igniter)
Expand Down
43 changes: 40 additions & 3 deletions lib/phoenix_kit/install/application_supervisor.ex
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,55 @@ defmodule PhoenixKit.Install.ApplicationSupervisor do
@moduledoc """
Installation helper for adding PhoenixKit supervisor to parent application.
Used by `mix phoenix_kit.install` task.

## Important

PhoenixKit.Supervisor MUST start AFTER the Ecto Repo because it depends on
the database for loading Settings cache and OAuth configuration.

Incorrect order (will crash):
```elixir
children = [
PhoenixKit.Supervisor, # ❌ Tries to read Settings from DB
MyApp.Repo # ⚠️ DB not ready yet!
]
```

Correct order:
```elixir
children = [
MyApp.Repo, # ✅ Start DB first
PhoenixKit.Supervisor # ✅ Then PhoenixKit
]
```
"""
use PhoenixKit.Install.IgniterCompat

alias Igniter.Libs.Ecto
alias Igniter.Libs.Phoenix
alias Igniter.Project.Application

def add_supervisor(igniter) do
{igniter, endpoint} = Phoenix.select_endpoint(igniter)
{igniter, repos} = Ecto.list_repos(igniter)

repo = List.first(repos)

# Build positioning options based on whether we found a Repo
opts =
case repo do
nil ->
# No Repo found - just add before endpoint
# User will need to manually reorder if they have a Repo
[before: [endpoint]]

repo_module ->
# Repo found - explicitly position AFTER Repo AND BEFORE Endpoint
# This ensures correct startup order: Repo → PhoenixKit → Endpoint
[after: [repo_module], before: [endpoint]]
end

igniter
|> Application.add_new_child(PhoenixKit.Supervisor,
before: [endpoint]
)
|> Application.add_new_child(PhoenixKit.Supervisor, opts)
end
end
Loading
Loading