From 2e0de1d518370340b65afa59b00da779aba08f86 Mon Sep 17 00:00:00 2001 From: szafranski Date: Sat, 13 Jun 2026 23:12:01 +0200 Subject: [PATCH] fix(agents): add include_disabled query to GET /api/agents Disabling a custom agent removed it from every UI surface, including the settings screen that hosts the re-enable toggle, because GET /api/agents only returns rows where enabled && available. Add an opt-in include_disabled query parameter (default false) that also re-surfaces rows hidden solely because the user disabled them while their spawn command still resolves on PATH. CLI-missing rows stay hidden in both modes. The default view is unchanged, so pickers keep showing only spawnable agents. Separate the disabled guard out of probe_resolved_command into a new probe_command helper so a disabled-but-installed row can be told apart from a disabled-and-missing one without changing the available field. --- crates/aionui-ai-agent/src/registry.rs | 159 +++++++++++++++++++ crates/aionui-ai-agent/src/routes/agent.rs | 13 +- crates/aionui-ai-agent/src/services/agent.rs | 11 +- crates/aionui-api-types/src/custom_agent.rs | 13 ++ crates/aionui-api-types/src/lib.rs | 3 +- 5 files changed, 192 insertions(+), 7 deletions(-) diff --git a/crates/aionui-ai-agent/src/registry.rs b/crates/aionui-ai-agent/src/registry.rs index 5835a5bc1..49b71ce22 100644 --- a/crates/aionui-ai-agent/src/registry.rs +++ b/crates/aionui-ai-agent/src/registry.rs @@ -255,6 +255,29 @@ impl AgentRegistry { rows } + /// Like [`Self::list_all`] but, when `include_disabled` is set, also + /// re-surfaces rows hidden *solely* because the user disabled them + /// (`enabled = 0`) whose spawn command still resolves on `$PATH`. + /// + /// This is the "manage agents" settings view: a user-disabled custom + /// agent must stay listed (greyed, with a working re-enable toggle) + /// instead of vanishing from the only surface that can turn it back + /// on. Rows hidden because the binary is missing stay hidden in both + /// modes — we never advertise an unusable vendor. With + /// `include_disabled = false` this is identical to [`Self::list_all`]. + pub async fn list_for_view(&self, include_disabled: bool) -> Vec { + let mut rows: Vec = self + .by_id + .read() + .await + .values() + .filter(|m| is_visible(m) || (include_disabled && is_disabled_but_installed(m))) + .cloned() + .collect(); + rows.sort_by(|a, b| a.sort_order.cmp(&b.sort_order).then_with(|| a.name.cmp(&b.name))); + rows + } + /// Unfiltered snapshot — used by internal paths that legitimately /// need to see user-disabled or missing rows (e.g. the UI's /// "manage agents" surface). Keep external API handlers on @@ -312,6 +335,17 @@ fn is_visible(meta: &AgentMetadata) -> bool { meta.enabled && meta.available } +/// A row that is hidden *only* because the user toggled it off, but is +/// otherwise installed and spawnable. Note we cannot key off +/// `meta.available`: [`probe_resolved_command`] short-circuits to +/// `Disabled` for `!enabled` rows, so a disabled row always carries +/// `available = false` regardless of whether its binary is present. +/// We therefore re-probe the command via [`probe_command`], which +/// skips the disabled guard and reports only the binary/runtime state. +fn is_disabled_but_installed(meta: &AgentMetadata) -> bool { + !meta.enabled && probe_command(meta).is_ok() +} + /// Turn a DB row into the public `AgentMetadata`, probing the command /// on disk so `available` reflects the current PATH state. Returns /// the probe reason alongside the row so the caller can log a single @@ -595,7 +629,18 @@ fn probe_resolved_command(meta: &AgentMetadata) -> Result Result { if meta.agent_source == AgentSource::Builtin && let Some(backend) = meta.backend.as_deref() && let Some(tool) = ManagedAcpToolId::from_backend(backend) @@ -745,6 +790,120 @@ mod tests { ); } + /// Insert a custom ACP agent row with the given spawn command and + /// enabled flag, then rehydrate so the registry recomputes + /// `available`. `command` is probed against the test host's `$PATH`. + async fn insert_custom_agent(reg: &Arc, id: &str, command: &str, enabled: bool) { + let params = aionui_db::UpsertAgentMetadataParams { + id, + icon: None, + name: id, + name_i18n: None, + description: Some("custom test agent"), + description_i18n: None, + backend: Some("custom"), + agent_type: "acp", + agent_source: "custom", + agent_source_info: None, + enabled, + command: Some(command), + args: Some("[]"), + env: Some("[]"), + native_skills_dirs: None, + behavior_policy: None, + yolo_id: None, + agent_capabilities: None, + auth_methods: None, + config_options: None, + available_modes: None, + available_models: None, + available_commands: None, + sort_order: 100, + }; + reg.repo_handle().upsert(¶ms).await.unwrap(); + reg.invalidate_and_rehydrate().await.unwrap(); + } + + /// A user-disabled custom agent whose CLI is still installed must be + /// absent from the default (picker) view but present in the + /// `include_disabled` (settings) view. `sh` is guaranteed to be on + /// `$PATH` on every test host. + #[tokio::test] + async fn list_for_view_resurfaces_disabled_but_installed_rows() { + let reg = registry().await; + insert_custom_agent(®, "custom-disabled-installed", "sh", false).await; + + let default_view = reg.list_for_view(false).await; + assert!( + !default_view.iter().any(|m| m.id == "custom-disabled-installed"), + "disabled agent must stay hidden from the default/picker view" + ); + + let managed_view = reg.list_for_view(true).await; + let row = managed_view + .iter() + .find(|m| m.id == "custom-disabled-installed") + .expect("disabled-but-installed agent must resurface with include_disabled=true"); + // The row stays marked unavailable (probe short-circuits on the + // disabled guard); the renderer greys it off `enabled`, not + // `available`. + assert!(!row.enabled, "resurfaced row must report enabled = false"); + assert!(!row.available, "resurfaced disabled row keeps available = false"); + } + + /// A custom agent whose binary is missing must stay hidden in *both* + /// views — `include_disabled` only re-surfaces user-disabled rows + /// that are otherwise installed, never uninstalled ones. + #[tokio::test] + async fn list_for_view_keeps_cli_missing_rows_hidden() { + let reg = registry().await; + insert_custom_agent( + ®, + "custom-disabled-missing", + "definitely-not-a-real-binary-xyz", + false, + ) + .await; + + assert!( + !reg.list_for_view(false) + .await + .iter() + .any(|m| m.id == "custom-disabled-missing"), + "CLI-missing row must stay hidden in the default view" + ); + assert!( + !reg.list_for_view(true) + .await + .iter() + .any(|m| m.id == "custom-disabled-missing"), + "CLI-missing row must stay hidden even with include_disabled=true" + ); + } + + /// An enabled + installed custom agent is present in both views and + /// re-enabling restores it everywhere — the picker contract. + #[tokio::test] + async fn list_for_view_includes_enabled_installed_rows_in_both_views() { + let reg = registry().await; + insert_custom_agent(®, "custom-enabled-installed", "sh", true).await; + + assert!( + reg.list_for_view(false) + .await + .iter() + .any(|m| m.id == "custom-enabled-installed"), + "enabled + installed agent must appear in the default view" + ); + assert!( + reg.list_for_view(true) + .await + .iter() + .any(|m| m.id == "custom-enabled-installed"), + "enabled + installed agent must appear in the management view" + ); + } + #[tokio::test] async fn list_by_agent_type_counts_seed_rows() { // Seed counts — exercised against the unfiltered view because diff --git a/crates/aionui-ai-agent/src/routes/agent.rs b/crates/aionui-ai-agent/src/routes/agent.rs index 926d366ec..e8f25e7cc 100644 --- a/crates/aionui-ai-agent/src/routes/agent.rs +++ b/crates/aionui-ai-agent/src/routes/agent.rs @@ -10,13 +10,13 @@ use axum::Router; use axum::extract::rejection::JsonRejection; -use axum::extract::{Extension, Json, Path, State}; +use axum::extract::{Extension, Json, Path, Query, State}; use axum::routing::{get, patch, post, put}; use aionui_api_types::{ AcpHealthCheckRequest, AcpHealthCheckResponse, AgentMetadata, ApiResponse, CustomAgentUpsertRequest, - DeleteCustomAgentResponse, ProviderHealthCheckRequest, ProviderHealthCheckResponse, SetEnabledRequest, - TryConnectCustomAgentRequest, TryConnectCustomAgentResponse, + DeleteCustomAgentResponse, ListAgentsQuery, ProviderHealthCheckRequest, ProviderHealthCheckResponse, + SetEnabledRequest, TryConnectCustomAgentRequest, TryConnectCustomAgentResponse, }; use aionui_auth::CurrentUser; use aionui_common::ApiError; @@ -40,9 +40,14 @@ pub fn agent_routes(state: AgentRouterState) -> Router { async fn list_agents( State(state): State, Extension(_user): Extension, + Query(query): Query, ) -> Result>>, ApiError> { Ok(Json(ApiResponse::ok( - state.service.list_agents().await.map_err(agent_error_to_api_error)?, + state + .service + .list_agents(query.include_disabled) + .await + .map_err(agent_error_to_api_error)?, ))) } diff --git a/crates/aionui-ai-agent/src/services/agent.rs b/crates/aionui-ai-agent/src/services/agent.rs index 677053564..c67059687 100644 --- a/crates/aionui-ai-agent/src/services/agent.rs +++ b/crates/aionui-ai-agent/src/services/agent.rs @@ -69,10 +69,17 @@ impl AgentService { // Agent operations impl AgentService { - pub async fn list_agents(&self) -> Result, AgentError> { + /// List agents for `GET /api/agents`. + /// + /// `include_disabled` is the opt-in management view: when set, rows + /// hidden solely because the user disabled them (but still installed) + /// are re-surfaced so the Agent settings screen can show them greyed + /// with a working re-enable toggle. Pickers call this with `false` + /// and keep seeing only spawnable agents. + pub async fn list_agents(&self, include_disabled: bool) -> Result, AgentError> { Ok(self .registry - .list_all() + .list_for_view(include_disabled) .await .into_iter() .filter(|agent| agent.agent_type.supports_new_conversation()) diff --git a/crates/aionui-api-types/src/custom_agent.rs b/crates/aionui-api-types/src/custom_agent.rs index 0a3f88d50..5e2bbf901 100644 --- a/crates/aionui-api-types/src/custom_agent.rs +++ b/crates/aionui-api-types/src/custom_agent.rs @@ -51,6 +51,19 @@ pub struct SetEnabledRequest { pub enabled: bool, } +/// Query parameters for `GET /api/agents`. +/// +/// `include_disabled` is the opt-in management view used only by the +/// Agent settings screen: when `true`, agents hidden solely because the +/// user disabled them (but still installed) are re-surfaced so they stay +/// listed with a working re-enable toggle. Defaults to `false`, which is +/// the picker-safe filtered view. +#[derive(Debug, Clone, Default, Deserialize)] +pub struct ListAgentsQuery { + #[serde(default)] + pub include_disabled: bool, +} + /// Response body for `DELETE /api/agents/custom/{id}`. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct DeleteCustomAgentResponse { diff --git a/crates/aionui-api-types/src/lib.rs b/crates/aionui-api-types/src/lib.rs index 4c05c4eee..bb9937373 100644 --- a/crates/aionui-api-types/src/lib.rs +++ b/crates/aionui-api-types/src/lib.rs @@ -78,7 +78,8 @@ pub use cron::{ ListCronJobsQuery, RunNowResponse, SaveCronSkillRequest, UpdateCronJobRequest, }; pub use custom_agent::{ - CustomAgentAdvancedOverrides, CustomAgentUpsertRequest, DeleteCustomAgentResponse, SetEnabledRequest, + CustomAgentAdvancedOverrides, CustomAgentUpsertRequest, DeleteCustomAgentResponse, ListAgentsQuery, + SetEnabledRequest, }; pub use extension::{ DisableExtensionRequest, EnableExtensionRequest, ExtensionSummaryResponse, GetI18nRequest, GetPermissionsRequest,