Apply review comments

Author: aleDsz

lib/livebook/apps.ex

@@ -76,6 +76,10 @@ defmodule Livebook.Apps do
 
   @doc """
   Returns if the given running app is authorized to given user.
+
+  Teams apps have authorization rules that can restrict access on
+  per-user basis. This kind of authorization is not applicable to
+  other types of apps, for those this function always returns `true`.
   """
   @spec authorized?(App.t(), Livebook.Users.User.t()) :: boolean()
   def authorized?(app, user)
@@ -86,7 +90,7 @@ defmodule Livebook.Apps do
     Livebook.Hubs.TeamClient.user_app_access?(id, user.groups, slug)
   end
 
-  def authorized?(%{permanent: false}, _user), do: true
+  def authorized?(_app, _user), do: true
 
   @doc """
   Updates the given app info across the cluster.

lib/livebook/hubs.ex

@@ -124,28 +124,20 @@ defmodule Livebook.Hubs do
     if get_default_hub().id == hub_id, do: unset_default_hub(), else: :ok
   end
 
-  if Mix.env() == :test do
-    # In test environment, disconnect synchronously to avoid race conditions
-    # during test cleanup where other processes might still be accessing hubs
-    defp disconnect_hub(hub) do
-      Provider.disconnect(hub)
-    end
-  else
-    defp disconnect_hub(hub) do
-      # We use a task supervisor because the hub connection itself
-      # calls delete_hub (which calls this function), otherwise we deadlock.
-      Task.Supervisor.start_child(Livebook.TaskSupervisor, fn ->
-        # Since other processes may have been communicating
-        # with the hub, we don't want to terminate abruptly and
-        # make them crash, so we give it some time to shut down.
-        #
-        # The default backoff is 5.5s, so we round it down to 5s.
-        Process.sleep(30_000)
-        :ok = Provider.disconnect(hub)
-      end)
+  defp disconnect_hub(hub) do
+    # We use a task supervisor because the hub connection itself
+    # calls delete_hub (which calls this function), otherwise we deadlock.
+    Task.Supervisor.start_child(Livebook.TaskSupervisor, fn ->
+      # Since other processes may have been communicating
+      # with the hub, we don't want to terminate abruptly and
+      # make them crash, so we give it some time to shut down.
+      #
+      # The default backoff is 5.5s, so we round it down to 5s.
+      Process.sleep(30_000)
+      :ok = Provider.disconnect(hub)
+    end)
 
-      :ok
-    end
+    :ok
   end
 
   defp to_struct(%{id: "personal-" <> _} = fields) do

lib/livebook/hubs/team_client.ex

@@ -93,8 +93,6 @@ defmodule Livebook.Hubs.TeamClient do
   @spec get_deployment_groups(String.t()) :: list(Teams.DeploymentGroup.t())
   def get_deployment_groups(id) do
     GenServer.call(registry_name(id), :get_deployment_groups)
-  catch
-    :exit, _ -> []
   end
 
   @doc """
@@ -103,8 +101,6 @@ defmodule Livebook.Hubs.TeamClient do
   @spec get_app_deployments(String.t()) :: list(Teams.AppDeployment.t())
   def get_app_deployments(id) do
     GenServer.call(registry_name(id), :get_app_deployments)
-  catch
-    :exit, _ -> []
   end
 
   @doc """
@@ -134,8 +130,6 @@ defmodule Livebook.Hubs.TeamClient do
   @spec get_agents(String.t()) :: list(Teams.Agent.t())
   def get_agents(id) do
     GenServer.call(registry_name(id), :get_agents)
-  catch
-    :exit, _ -> []
   end
 
   @doc """
@@ -144,8 +138,6 @@ defmodule Livebook.Hubs.TeamClient do
   @spec identity_enabled?(String.t()) :: boolean()
   def identity_enabled?(id) do
     GenServer.call(registry_name(id), :identity_enabled?)
-  catch
-    :exit, _ -> false
   end
 
   @doc """
@@ -154,8 +146,6 @@ defmodule Livebook.Hubs.TeamClient do
   @spec get_environment_variables(String.t()) :: list(Teams.EnvironmentVariable.t())
   def get_environment_variables(id) do
     GenServer.call(registry_name(id), :get_environment_variables)
-  catch
-    :exit, _ -> []
   end
 
   @doc """
@@ -164,8 +154,6 @@ defmodule Livebook.Hubs.TeamClient do
   @spec user_full_access?(String.t(), list(map())) :: boolean()
   def user_full_access?(id, groups) do
     GenServer.call(registry_name(id), {:check_full_access, groups})
-  catch
-    :exit, _ -> false
   end
 
   @doc """
@@ -174,8 +162,6 @@ defmodule Livebook.Hubs.TeamClient do
   @spec user_app_access?(String.t(), list(map()), String.t()) :: boolean()
   def user_app_access?(id, groups, slug) do
     GenServer.call(registry_name(id), {:check_app_access, groups, slug})
-  catch
-    :exit, _ -> false
   end
 
   @doc """