fix(agents): run custom registry agents with their real tools (flows + chat + tasks)

src/openhuman/agent/harness/definition.rs

@@ -662,6 +662,14 @@ pub enum DefinitionSource {
     Builtin,
     /// Loaded from a TOML file at the given absolute path.
     File(PathBuf),
+    /// Synthesized at lookup time from a user-authored
+    /// [`AgentRegistryEntry`](crate::openhuman::agent_registry::AgentRegistryEntry)
+    /// (`AgentRegistrySource::Custom`) by `agent_registry::defaults::definition_from_registry_entry`.
+    /// Never persisted in the [`AgentDefinitionRegistry`] — built fresh per
+    /// factory call so config edits take effect immediately (closes the gap
+    /// where custom agents ran persona-only instead of with their real tool
+    /// belt).
+    CustomRegistry,
 }
 
 // ─────────────────────────────────────────────────────────────────────────────

src/openhuman/agent/harness/session/builder/builder_tests.rs

@@ -241,6 +241,108 @@ async fn build_session_agent_falls_back_to_global_default_when_no_definition() {
     );
 }
 
+// ─────────────────────────────────────────────────────────────────────────────
+// B38 (Gap 2) — a custom (non-shipped) `AgentRegistryEntry` must synthesize a
+// real `AgentDefinition` and run with its own `ToolScope::Named` filter,
+// instead of the factory hard-erroring "agent definition '…' not found in
+// registry" (chat / task-dispatcher) because it never consulted
+// `config.agent_registry.entries`.
+//
+// Regression note: this test deliberately does NOT call
+// `AgentDefinitionRegistry::init_global*` itself, so — depending on whether
+// an earlier test in this binary already initialised the process-wide
+// `OnceLock` singleton — it exercises `build_session_agent_inner`'s tool-
+// visibility computation under EITHER state: `(Some(def), Some(registry))`
+// or `(Some(def), None)`. Both arms must apply `def.tools` (the synthesized
+// `ToolScope::Named` from `definition_from_registry_entry`); the `None`
+// (registry-uninitialized) arm previously fell through to the catch-all
+// "no registry, no filter" case and silently discarded the custom agent's
+// allowlist, leaving `visible_tool_names_for_test()` empty. See the
+// `(Some(def), None)` match arm in `factory.rs`'s delegation-tool-and-
+// visibility block for the fix.
+// ─────────────────────────────────────────────────────────────────────────────
+
+#[tokio::test]
+async fn from_config_for_agent_synthesizes_custom_registry_entry_with_named_scope() {
+    use crate::openhuman::agent::harness::session::types::Agent;
+    use crate::openhuman::agent_registry::types::{
+        AgentRegistryEntry, AgentRegistrySource, AgentSubagentPolicy,
+    };
+    use crate::openhuman::tokenjuice::RETRIEVE_TOOL_NAME;
+
+    let tmp = tempfile::TempDir::new().unwrap();
+    let mut config = test_config(&tmp);
+    config.agent_registry.entries = vec![AgentRegistryEntry {
+        id: "finance_analyst_b38".to_string(),
+        name: "Finance Analyst".to_string(),
+        description: "Reviews spend and drafts finance summaries.".to_string(),
+        source: AgentRegistrySource::Custom,
+        enabled: true,
+        model: Some("hint:reasoning".to_string()),
+        system_prompt: Some("You are a meticulous finance analyst.".to_string()),
+        tool_allowlist: vec!["memory_search".to_string(), "web_search".to_string()],
+        tool_denylist: Vec::new(),
+        subagents: AgentSubagentPolicy::default(),
+        tags: Vec::new(),
+        metadata: serde_json::Value::Null,
+    }];
+
+    // Precondition: this id must NOT be a harness definition (built-in or
+    // workspace TOML) — the whole point is that only the config-backed
+    // custom registry knows about it.
+    assert!(
+        crate::openhuman::agent::harness::definition::AgentDefinitionRegistry::global()
+            .map(|reg| reg.get("finance_analyst_b38").is_none())
+            .unwrap_or(true),
+        "test id must not collide with a real harness definition"
+    );
+
+    let agent = Agent::from_config_for_agent(&config, "finance_analyst_b38").expect(
+        "a custom agent_registry entry must synthesize a real AgentDefinition and build \
+         successfully instead of erroring",
+    );
+
+    let visible = agent.visible_tool_names_for_test();
+    assert!(
+        visible.contains("memory_search") && visible.contains("web_search"),
+        "the custom agent's tool_allowlist must become a real ToolScope::Named filter: {visible:?}"
+    );
+    assert!(
+        visible.contains(RETRIEVE_TOOL_NAME),
+        "the compaction recovery tool must join any non-empty Named allowlist: {visible:?}"
+    );
+    assert!(
+        !visible.contains("automate"),
+        "a tool outside the custom agent's allowlist must not be visible: {visible:?}"
+    );
+}
+
+#[tokio::test]
+async fn from_config_for_agent_still_errors_for_a_genuinely_unknown_id() {
+    use crate::openhuman::agent::harness::session::types::Agent;
+
+    let tmp = tempfile::TempDir::new().unwrap();
+    let config = test_config(&tmp);
+
+    // No harness definition AND no config.agent_registry entry for this id —
+    // the factory must still hard-error rather than silently building an
+    // unfiltered/legacy agent.
+    //
+    // Note: `Agent` intentionally has no `Debug` impl (it holds `Box<dyn
+    // Tool>` / provider trait objects), so this must use `match` +
+    // `.is_err()` rather than `.expect_err()`, which requires `T: Debug`.
+    let result = Agent::from_config_for_agent(&config, "totally_unknown_agent_id_b38");
+    assert!(
+        result.is_err(),
+        "an id with no harness definition and no custom entry must error"
+    );
+    let err = result.err().unwrap();
+    assert!(
+        err.to_string().contains("totally_unknown_agent_id_b38"),
+        "error should name the unresolved agent id: {err}"
+    );
+}
+
 // ── #5050 Fix 1: shared `Arc<Config>` for the per-build tool config ──────────
 
 #[test]

src/openhuman/agent/harness/session/builder/factory.rs

@@ -74,47 +74,11 @@ impl Agent {
     pub fn from_config_for_agent(config: &Config, agent_id: &str) -> Result<Self> {
         // Look up the target definition up front so we can fail fast
         // with a clear error instead of building half an agent and then
-        // discovering the id is unknown. The registry is a singleton
-        // initialised at startup; if it's not yet populated we
-        // conservatively fall back to the legacy "orchestrator-shaped"
-        // build by proceeding without a definition override.
-        let target_def: Option<crate::openhuman::agent::harness::definition::AgentDefinition> =
-            match AgentDefinitionRegistry::global() {
-                Some(reg) => match reg.get(agent_id) {
-                    Some(def) => Some(def.clone()),
-                    None if agent_id == "orchestrator" => {
-                        // Orchestrator is allowed to be missing from the
-                        // registry (legacy path, tests, pre-startup) —
-                        // fall back to default behaviour.
-                        log::debug!(
-                            "[agent::builder] orchestrator definition not in registry — \
-                         using legacy default prompt + filter"
-                        );
-                        None
-                    }
-                    None => {
-                        return Err(anyhow::anyhow!(
-                            "agent definition '{}' not found in registry",
-                            agent_id
-                        ));
-                    }
-                },
-                None => {
-                    if agent_id != "orchestrator" {
-                        return Err(anyhow::anyhow!(
-                            "AgentDefinitionRegistry is not initialised — cannot \
-                         resolve agent '{}'. Call AgentDefinitionRegistry::init_global \
-                         at startup.",
-                            agent_id
-                        ));
-                    }
-                    log::debug!(
-                        "[agent::builder] registry not initialised, orchestrator requested — \
-                     using legacy default prompt + filter"
-                    );
-                    None
-                }
-            };
+        // discovering the id is unknown. See `resolve_target_definition`
+        // for the full resolution order (harness registry, then the
+        // config-backed custom agent registry, then the orchestrator's
+        // legacy pre-startup fallback).
+        let target_def = resolve_target_definition(config, agent_id)?;
 
         log::info!(
             "[agent::builder] building session agent id={} \
@@ -197,30 +161,7 @@ impl Agent {
         profile_prompt_suffix: Option<String>,
         profile: Option<&crate::openhuman::profiles::AgentProfile>,
     ) -> Result<Self> {
-        let target_def: Option<crate::openhuman::agent::harness::definition::AgentDefinition> =
-            match AgentDefinitionRegistry::global() {
-                Some(reg) => match reg.get(agent_id) {
-                    Some(def) => Some(def.clone()),
-                    None if agent_id == "orchestrator" => None,
-                    None => {
-                        return Err(anyhow::anyhow!(
-                            "agent definition '{}' not found in registry",
-                            agent_id
-                        ));
-                    }
-                },
-                None => {
-                    if agent_id != "orchestrator" {
-                        return Err(anyhow::anyhow!(
-                            "AgentDefinitionRegistry is not initialised — cannot \
-                         resolve agent '{}'. Call AgentDefinitionRegistry::init_global \
-                         at startup.",
-                            agent_id
-                        ));
-                    }
-                    None
-                }
-            };
+        let target_def = resolve_target_definition(config, agent_id)?;
         Self::build_session_agent_inner(
             config,
             agent_id,
@@ -885,7 +826,35 @@ impl Agent {
                 };
                 (synthed, None)
             }
-            (_, None) => {
+            (Some(def), None) => {
+                // We have a target definition (either a pre-populated
+                // harness entry looked up before the registry singleton
+                // existed, or — the common case today — a `CustomRegistry`
+                // definition `resolve_target_definition` synthesizes
+                // straight from `config.agent_registry.entries` without
+                // ever consulting `AgentDefinitionRegistry::global()`, see
+                // `agent_registry::find_custom_in_config`). Delegation-tool
+                // synthesis needs the registry (to resolve named
+                // subagents), so it's skipped here, but `def.tools` is a
+                // real scope the caller authored and MUST still gate
+                // visibility — silently dropping it into the `(_, None)`
+                // "no registry, no filter" catch-all would leave a custom
+                // agent's `ToolScope::Named` allowlist entirely
+                // unenforced (visible tools empty rather than the named
+                // set), regressing the least-privilege contract this
+                // synthesis path exists to provide.
+                log::debug!(
+                    "[agent::builder] AgentDefinitionRegistry not initialised — skipping \
+                     delegation tool synthesis, but still applying target definition's own \
+                     tool scope"
+                );
+                let filter: Option<std::collections::HashSet<String>> = match &def.tools {
+                    ToolScope::Named(names) => Some(names.iter().cloned().collect()),
+                    ToolScope::Wildcard => None,
+                };
+                (Vec::new(), filter)
+            }
+            (None, None) => {
                 log::debug!(
                     "[agent::builder] AgentDefinitionRegistry not initialised — \
                      skipping delegation tool synthesis"
@@ -1214,6 +1183,81 @@ impl Agent {
     }
 }
 
+/// Resolves the `AgentDefinition` a session should be built from, given the
+/// requested `agent_id`, in three steps:
+///
+/// 1. **Harness registry** (`AgentDefinitionRegistry`, the process-global
+///    singleton of built-in + workspace-TOML-override definitions) — a hit
+///    here wins outright.
+/// 2. **Config-backed custom agent registry** (`config.agent_registry.entries`,
+///    `AgentRegistrySource::Custom`) — on a harness-registry miss (or the
+///    registry not yet being initialised), a user-authored custom agent is
+///    synthesized into a real `AgentDefinition` via
+///    `agent_registry::definition_from_registry_entry` so it runs through
+///    the exact same `build_session_agent_inner` path (and therefore the
+///    exact same `SecurityPolicy` / tool-filtering / approval gate) as a
+///    built-in. This closes the gap where a custom agent either hard-errored
+///    (chat, task-dispatcher) or silently ran tool-less/persona-only (flows'
+///    `RegistryFallback`) — see the cross-cutting fix in the PR that added
+///    this function.
+/// 3. **Orchestrator legacy fallback** — `orchestrator` alone is allowed to
+///    resolve to `None` (pre-startup, tests): the caller then builds with the
+///    default prompt/filter, matching pre-#1 behaviour.
+///
+/// Any other id that resolves nowhere is a hard error, exactly as before this
+/// function existed — only the *search order* changed, not the failure
+/// contract for a genuinely-unknown id.
+fn resolve_target_definition(
+    config: &Config,
+    agent_id: &str,
+) -> Result<Option<crate::openhuman::agent::harness::definition::AgentDefinition>> {
+    let registry = AgentDefinitionRegistry::global();
+
+    if let Some(reg) = registry {
+        if let Some(def) = reg.get(agent_id) {
+            return Ok(Some(def.clone()));
+        }
+    }
+
+    // Harness registry miss (or not yet initialised). Before failing, check
+    // the config-backed custom agent registry — the one place custom
+    // (non-shipped) agents live.
+    if let Some(entry) = crate::openhuman::agent_registry::find_custom_in_config(config, agent_id) {
+        log::info!(
+            "[agent::builder] agent_id={} not found in the harness AgentDefinitionRegistry — \
+             synthesizing a definition from its custom agent_registry entry so it runs with its \
+             real tool belt instead of persona-only / erroring",
+            agent_id
+        );
+        return Ok(Some(
+            crate::openhuman::agent_registry::definition_from_registry_entry(&entry),
+        ));
+    }
+
+    if agent_id == "orchestrator" {
+        // Orchestrator is allowed to be missing from every source (legacy
+        // path, tests, pre-startup) — fall back to default behaviour.
+        log::debug!(
+            "[agent::builder] orchestrator definition not in any registry — using legacy \
+             default prompt + filter"
+        );
+        return Ok(None);
+    }
+
+    if registry.is_none() {
+        return Err(anyhow::anyhow!(
+            "AgentDefinitionRegistry is not initialised — cannot resolve agent '{}'. Call \
+             AgentDefinitionRegistry::init_global at startup.",
+            agent_id
+        ));
+    }
+
+    Err(anyhow::anyhow!(
+        "agent definition '{}' not found in registry",
+        agent_id
+    ))
+}
+
 /// Resolve the `Config` the tool registry is built from (#5050, Fix 1).
 ///
 /// Normally this is the shared `base_config` — returned as a refcount bump, not a

src/openhuman/agent/library/ops.rs

@@ -59,7 +59,9 @@ pub fn metadata_from_definition(def: &AgentDefinition) -> AgentDefinitionDisplay
         write_capable: is_write_capable(def),
         source: match &def.source {
             DefinitionSource::Builtin => AgentDefinitionSource::Builtin,
-            DefinitionSource::File(_) => AgentDefinitionSource::Custom,
+            DefinitionSource::File(_) | DefinitionSource::CustomRegistry => {
+                AgentDefinitionSource::Custom
+            }
         },
     }
 }