From 0d62e8489fdcaa0ffb73c2afb1803a0be4601a90 Mon Sep 17 00:00:00 2001 From: Mega Mind <146339422+M3gA-Mind@users.noreply.github.com> Date: Mon, 25 May 2026 22:58:57 +0530 Subject: [PATCH] fix(inference): preserve connected-app tools when BYOK provider is configured (#2632) --- app/src/services/api/aiSettingsApi.ts | 20 ++ app/test/e2e/specs/mega-flow.spec.ts | 34 ++- .../agent/agents/orchestrator/prompt.rs | 13 + .../agent/harness/session/builder.rs | 7 + .../agent/harness/subagent_runner/ops.rs | 17 +- src/openhuman/inference/provider/factory.rs | 73 ++++- .../inference/provider/factory_test.rs | 273 ++++++++++++++++++ 7 files changed, 423 insertions(+), 14 deletions(-) diff --git a/app/src/services/api/aiSettingsApi.ts b/app/src/services/api/aiSettingsApi.ts index 0c65599e2..40339ad97 100644 --- a/app/src/services/api/aiSettingsApi.ts +++ b/app/src/services/api/aiSettingsApi.ts @@ -226,6 +226,26 @@ export async function loadAISettings(): Promise { subconscious: parseProviderString(config.subconscious_provider), }; + // Diagnostic: detect partial BYOK routing — some workloads have a BYOK cloud + // provider configured while others are left at default/openhuman. The Rust + // factory inherits the BYOK provider for unset workloads, but this log makes + // it easy to trace the config state from the frontend side. + const byokProvider = (['chat', 'reasoning', 'agentic', 'coding'] as const).find(w => { + const ref_ = routing[w]; + return ref_.kind === 'cloud'; + }); + const hasUnsetChatWorkloads = (['chat', 'reasoning', 'coding'] as const).some(w => { + const ref_ = routing[w]; + return ref_.kind === 'default'; + }); + if (byokProvider !== undefined && hasUnsetChatWorkloads) { + const byokSlug = (routing[byokProvider] as { kind: 'cloud'; providerSlug: string }) + .providerSlug; + console.debug( + '[ai-settings] partial BYOK routing detected — unset workloads will inherit from: ' + byokSlug + ); + } + return { cloudProviders, routing }; } // ─── Write path: diff + save ─────────────────────────────────────────────── diff --git a/app/test/e2e/specs/mega-flow.spec.ts b/app/test/e2e/specs/mega-flow.spec.ts index 5c3d66050..db9b2852f 100644 --- a/app/test/e2e/specs/mega-flow.spec.ts +++ b/app/test/e2e/specs/mega-flow.spec.ts @@ -70,6 +70,27 @@ async function waitForMockRequest( return undefined; } +/** + * Poll the core's `auth_get_session_token` RPC until it returns a non-null + * token, confirming that `auth_store_session` has fully written the JWT to + * disk. This is more reliable than waiting for the mock's `/auth/me` log + * entry: that entry is recorded when the request *arrives* at the mock, but + * `store_session` finishes writing only after the response is received and + * the auth-profile file is flushed. + */ +async function waitForCoreSessionToken(timeoutMs = 12_000): Promise { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const snap = await callOpenhumanRpc('openhuman.auth_get_session_token', {}); + // RpcOutcome wraps the payload: json.result = { result: { token }, logs } + const token = snap.result?.result?.token ?? snap.result?.token; + if (snap.ok && token) return; + await browser.pause(300); + } + console.warn(`${LOG} waitForCoreSessionToken: session token not written within ${timeoutMs}ms`); +} + async function resetEverything(label: string): Promise { console.log(`${LOG} reset (${label}) — admin reset only (skip destructive core reset)`); // Mock-side reset is enough to give each scenario a clean slate for the @@ -237,9 +258,11 @@ describe('Mega flow — login + Gmail OAuth + Composio in one session', () => { // Re-login since reset wipes the session. await triggerDeepLink('openhuman://auth?token=mega-composio-token'); await waitForMockRequest('POST', '/telegram/login-tokens/', 15_000); - // Wait for the core to finish storing the session token before calling - // composio RPCs — without this the session may not be persisted yet. - await waitForMockRequest('GET', '/auth/me', 10_000); + // Poll until the core has written the session JWT to disk. The mock's + // /auth/me log entry fires when the request *arrives* (during + // store_session's token validation), which is before the profile file + // is flushed — so we need a deeper signal here. + await waitForCoreSessionToken(12_000); // Seed connections + available triggers; start with an empty active list. setMockBehaviors({ @@ -566,8 +589,9 @@ describe('Mega flow — login + Gmail OAuth + Composio in one session', () => { await triggerDeepLink('openhuman://auth?token=mega-composio-webhook-token'); await waitForMockRequest('POST', '/telegram/login-tokens/', 15_000); - // Wait for the core to finish storing the session token before proceeding. - await waitForMockRequest('GET', '/auth/me', 10_000); + // Poll until the core has written the session JWT to disk — same fix as + // Scenario 4; see waitForCoreSessionToken for the full explanation. + await waitForCoreSessionToken(12_000); clearRequestLog(); // Seed composio state. diff --git a/src/openhuman/agent/agents/orchestrator/prompt.rs b/src/openhuman/agent/agents/orchestrator/prompt.rs index 2ebcb19ae..0bde1cd77 100644 --- a/src/openhuman/agent/agents/orchestrator/prompt.rs +++ b/src/openhuman/agent/agents/orchestrator/prompt.rs @@ -91,6 +91,10 @@ fn render_delegation_guide(integrations: &[ConnectedIntegration]) -> String { } let mut out = String::from( "## Connected Integrations\n\n\ + IMPORTANT: You MUST use the `delegate_to_integrations_agent` tool for any request \ + involving connected services. You do NOT have direct access to these services — all \ + interaction must go through delegation. Never claim you cannot access a connected \ + service without first attempting delegation.\n\n\ The following services have an active connection. Their tool implementations \ live inside the `integrations_agent` sub-agent — NOT in your own tool list. \ Delegate with `delegate_to_integrations_agent`, passing the toolkit slug as \ @@ -249,6 +253,15 @@ mod tests { assert!(!body.contains("spawn_subagent(agent_id=\"integrations_agent\"")); // Delegator voice must NOT use the skill-executor wording. assert!(!body.contains("You have direct access")); + // Must contain the hardened delegation instruction. + assert!( + body.contains("IMPORTANT"), + "delegation guide must contain the IMPORTANT instruction" + ); + assert!( + body.contains("Never claim you cannot access a connected service without first attempting delegation"), + "delegation guide must instruct the model to always attempt delegation" + ); } #[test] diff --git a/src/openhuman/agent/harness/session/builder.rs b/src/openhuman/agent/harness/session/builder.rs index 1acd0c30d..5b8b42008 100644 --- a/src/openhuman/agent/harness/session/builder.rs +++ b/src/openhuman/agent/harness/session/builder.rs @@ -924,6 +924,13 @@ impl Agent { }; let (provider, mut model_name): (Box, String) = crate::openhuman::inference::provider::create_chat_provider(provider_role, config)?; + log::info!( + "[session-builder] agent_id={} provider_role={} resolved_model={} supports_native_tools={}", + agent_id, + provider_role, + model_name, + provider.supports_native_tools() + ); let target_agent_id = target_def .map(|def| def.id.as_str()) .unwrap_or("orchestrator"); diff --git a/src/openhuman/agent/harness/subagent_runner/ops.rs b/src/openhuman/agent/harness/subagent_runner/ops.rs index 048bd1f9d..ba8712107 100644 --- a/src/openhuman/agent/harness/subagent_runner/ops.rs +++ b/src/openhuman/agent/harness/subagent_runner/ops.rs @@ -147,11 +147,20 @@ pub(super) fn resolve_subagent_provider( (std::sync::Arc::from(p), m) } Err(e) => { + let suggested_key = match workload.as_str() { + "summarization" | "memory" => "memory_provider".to_string(), + _ => format!("{workload}_provider"), + }; log::warn!( - "[subagent_runner] workload '{}' provider build failed ({}) for agent_id={} — \ - falling back to parent provider + parent model '{}'", - workload, e, agent_id, parent_model - ); + "[subagent_runner] workload='{}' provider build failed for agent_id={} error='{}' \ + falling back to parent provider (parent_model='{}'). \ + Consider setting {} in config.", + workload, + agent_id, + e, + parent_model, + suggested_key + ); (parent_provider, parent_model) } } diff --git a/src/openhuman/inference/provider/factory.rs b/src/openhuman/inference/provider/factory.rs index 25c24cfb6..12329e01d 100644 --- a/src/openhuman/inference/provider/factory.rs +++ b/src/openhuman/inference/provider/factory.rs @@ -97,11 +97,21 @@ pub(crate) fn is_known_openhuman_tier(model: &str) -> bool { /// Return the configured provider string for a named workload role. /// -/// Empty / `"cloud"` resolves through `primary_cloud`. For backwards -/// compatibility, a legacy external `inference_url` takes precedence when -/// `primary_cloud` still points at OpenHuman because migration 1→2 preserved -/// the URL as a custom provider entry but older configs did not explicitly set -/// per-workload routes. +/// Empty / `"cloud"` resolves through BYOK fallback first for the three +/// chat-tier roles (`chat`, `reasoning`, `coding`), then `primary_cloud`. +/// When a BYOK cloud provider is detected on any workload, unset chat-tier +/// routes inherit it rather than silently falling back to the managed backend. +/// +/// Only `chat`, `reasoning`, and `coding` participate in BYOK inheritance. +/// Background workloads (`memory`, `embeddings`, `heartbeat`, `learning`, +/// `subconscious`) and the `agentic` workload always fall through to +/// `primary_cloud` — they use tier-specific models that BYOK providers don't +/// understand, and their providers are configured independently. +/// +/// For backwards compatibility, a legacy external `inference_url` takes +/// precedence when `primary_cloud` still points at OpenHuman because +/// migration 1→2 preserved the URL as a custom provider entry but older +/// configs did not explicitly set per-workload routes. pub fn provider_for_role(role: &str, config: &Config) -> String { let opt = match role { "chat" => config.chat_provider.as_deref(), @@ -122,12 +132,65 @@ pub fn provider_for_role(role: &str, config: &Config) -> String { }; let s = opt.unwrap_or("").trim(); if s.is_empty() || s == "cloud" { + // BYOK inheritance is scoped to the three chat-tier roles only. + // Background workloads (memory, embeddings, heartbeat, learning, + // subconscious) and the agentic workload must stay on the managed + // backend — they use tier-specific models that BYOK providers don't + // understand, and their providers are configured separately. + if matches!(role, "chat" | "reasoning" | "coding") { + if let Some(byok) = resolve_byok_fallback_provider_string(config) { + log::debug!( + "[providers][byok-fallback] role={} inheriting BYOK provider string={}", + role, + byok + ); + return byok; + } + } resolve_primary_cloud_provider_string(config) } else { s.to_string() } } +/// Find the first BYOK cloud provider string configured across all workload +/// routes, skipping local providers (ollama, lmstudio) and managed-backend +/// sentinels ("openhuman", "cloud", empty). +/// +/// Returns `None` when no BYOK cloud provider is configured, in which case +/// the caller should fall through to `resolve_primary_cloud_provider_string`. +/// +/// Priority order: chat → reasoning → agentic → coding (user-facing workloads +/// first so the most prominent setting wins for unset background workloads). +pub(crate) fn resolve_byok_fallback_provider_string(config: &Config) -> Option { + let candidates = [ + config.chat_provider.as_deref(), + config.reasoning_provider.as_deref(), + config.agentic_provider.as_deref(), + config.coding_provider.as_deref(), + ]; + for candidate in candidates.iter().flatten() { + let s = candidate.trim(); + if s.is_empty() || s == "cloud" || s == PROVIDER_OPENHUMAN { + continue; + } + // Skip local providers — they are not suitable fallbacks for agentic + // or background workloads that run on the managed backend. + if s.starts_with(OLLAMA_PROVIDER_PREFIX) || s.starts_with(LM_STUDIO_PROVIDER_PREFIX) { + continue; + } + // Any remaining non-empty string with a colon is a BYOK cloud slug. + if s.contains(':') { + log::debug!( + "[providers][byok-fallback] resolve_byok_fallback found candidate={}", + s + ); + return Some(s.to_string()); + } + } + None +} + /// Build a `(Provider, model)` for the given workload role. pub fn create_chat_provider( role: &str, diff --git a/src/openhuman/inference/provider/factory_test.rs b/src/openhuman/inference/provider/factory_test.rs index cdbab448e..05fb5c866 100644 --- a/src/openhuman/inference/provider/factory_test.rs +++ b/src/openhuman/inference/provider/factory_test.rs @@ -883,6 +883,279 @@ fn byok_sentinel_error_mentions_configuration_action() { ); } +// ── BYOK workload inheritance tests ────────────────────────────────────────── + +#[test] +fn byok_fallback_agentic_always_uses_managed_backend() { + // The agentic role is excluded from BYOK inheritance: it uses managed-backend + // tier models (agentic-v1) and handles hint:agentic routing directives. + let mut config = Config::default(); + config.cloud_providers.push(openai_entry("p_oai", "openai")); + config.chat_provider = Some("openai:gpt-4o".to_string()); + // agentic_provider is unset and chat BYOK is configured → agentic must + // still resolve to the managed backend, NOT inherit from chat BYOK. + let result = provider_for_role("agentic", &config); + assert_eq!( + result, "openhuman", + "agentic role must always resolve to managed backend regardless of BYOK config" + ); +} + +#[test] +fn byok_fallback_inherits_chat_provider_for_unset_coding() { + let mut config = Config::default(); + config.cloud_providers.push(openai_entry("p_oai", "openai")); + config.chat_provider = Some("openai:gpt-4o".to_string()); + // coding_provider is unset → should inherit chat BYOK + let result = provider_for_role("coding", &config); + assert_eq!( + result, "openai:gpt-4o", + "unset coding must inherit chat BYOK" + ); + assert_ne!(result, "openhuman"); +} + +#[test] +fn byok_fallback_inherits_reasoning_when_chat_unset() { + let mut config = Config::default(); + config + .cloud_providers + .push(anthropic_entry("p_ant", "anthropic")); + config.reasoning_provider = Some("anthropic:claude-opus-4-7".to_string()); + // coding_provider is unset, chat_provider is unset → should inherit reasoning BYOK + let result = provider_for_role("coding", &config); + assert_eq!( + result, "anthropic:claude-opus-4-7", + "unset coding must inherit reasoning BYOK when chat is unset" + ); +} + +#[test] +fn byok_fallback_respects_priority_order() { + let mut config = Config::default(); + config.cloud_providers.push(openai_entry("p_oai", "openai")); + config + .cloud_providers + .push(anthropic_entry("p_ant", "anthropic")); + config.chat_provider = Some("openai:gpt-4o".to_string()); + config.reasoning_provider = Some("anthropic:claude-opus-4-7".to_string()); + // chat wins (higher priority) for unset coding + let result = provider_for_role("coding", &config); + assert_eq!( + result, "openai:gpt-4o", + "chat_provider must win over reasoning_provider in priority" + ); +} + +#[test] +fn byok_fallback_skips_local_ollama() { + let mut config = Config::default(); + config.chat_provider = Some("ollama:llama3.1".to_string()); + // Ollama is local — must NOT be inherited for non-agentic roles either + let result = provider_for_role("coding", &config); + assert_eq!( + result, "openhuman", + "local ollama must not be inherited as BYOK fallback" + ); +} + +#[test] +fn byok_fallback_skips_local_lmstudio() { + let mut config = Config::default(); + config.chat_provider = Some("lmstudio:google/gemma-4-e4b".to_string()); + // LM Studio is local — must NOT be inherited; fall through to openhuman + let result = provider_for_role("coding", &config); + assert_eq!( + result, "openhuman", + "local lmstudio must not be inherited as BYOK fallback" + ); +} + +#[test] +fn byok_fallback_skips_openhuman_sentinel() { + let mut config = Config::default(); + config.chat_provider = Some("openhuman".to_string()); + // "openhuman" is the managed backend sentinel, not BYOK + let result = provider_for_role("coding", &config); + assert_eq!( + result, "openhuman", + "openhuman sentinel in chat must not be treated as BYOK" + ); +} + +#[test] +fn byok_fallback_skips_cloud_sentinel() { + let mut config = Config::default(); + config.chat_provider = Some("cloud".to_string()); + // "cloud" means "use primary" — not BYOK + let result = provider_for_role("coding", &config); + assert_eq!( + result, "openhuman", + "cloud sentinel in chat must not be treated as BYOK" + ); +} + +#[test] +fn byok_fallback_no_byok_configured() { + // All workload routes unset → falls through to managed backend unchanged + let config = Config::default(); + assert_eq!( + provider_for_role("coding", &config), + "openhuman", + "no BYOK configured must fall through to openhuman for coding" + ); + assert_eq!( + provider_for_role("agentic", &config), + "openhuman", + "no BYOK configured must fall through to openhuman for agentic" + ); +} + +#[test] +fn byok_fallback_explicit_agentic_overrides_chat_byok() { + let mut config = Config::default(); + config.cloud_providers.push(openai_entry("p_oai", "openai")); + config + .cloud_providers + .push(anthropic_entry("p_ant", "anthropic")); + config.chat_provider = Some("openai:gpt-4o".to_string()); + config.agentic_provider = Some("anthropic:claude-haiku-4-5".to_string()); + // Explicit agentic setting wins over BYOK inheritance + let result = provider_for_role("agentic", &config); + assert_eq!( + result, "anthropic:claude-haiku-4-5", + "explicit agentic_provider must win over inherited BYOK" + ); +} + +#[test] +fn byok_fallback_explicit_openhuman_agentic_overrides_chat_byok() { + let mut config = Config::default(); + config.cloud_providers.push(openai_entry("p_oai", "openai")); + config.chat_provider = Some("openai:gpt-4o".to_string()); + config.agentic_provider = Some("openhuman".to_string()); + // Explicit "openhuman" in agentic wins — user made a deliberate choice + let result = provider_for_role("agentic", &config); + assert_eq!( + result, "openhuman", + "explicit openhuman in agentic must not be overridden by BYOK inheritance" + ); +} + +#[test] +fn byok_fallback_all_workloads_set_independently() { + let mut config = Config::default(); + config.cloud_providers.push(openai_entry("p_oai", "openai")); + config + .cloud_providers + .push(anthropic_entry("p_ant", "anthropic")); + config.chat_provider = Some("openai:gpt-4o".to_string()); + config.reasoning_provider = Some("anthropic:claude-opus-4-7".to_string()); + config.agentic_provider = Some("anthropic:claude-haiku-4-5".to_string()); + config.coding_provider = Some("openai:gpt-4o-mini".to_string()); + assert_eq!(provider_for_role("chat", &config), "openai:gpt-4o"); + assert_eq!( + provider_for_role("reasoning", &config), + "anthropic:claude-opus-4-7" + ); + assert_eq!( + provider_for_role("agentic", &config), + "anthropic:claude-haiku-4-5" + ); + assert_eq!(provider_for_role("coding", &config), "openai:gpt-4o-mini"); +} + +#[test] +fn byok_fallback_empty_string_treated_as_unset() { + let mut config = Config::default(); + config.cloud_providers.push(openai_entry("p_oai", "openai")); + config.chat_provider = Some("openai:gpt-4o".to_string()); + config.coding_provider = Some(String::new()); // empty string = unset + // Empty string must be treated as unset → coding inherits chat BYOK + let result = provider_for_role("coding", &config); + assert_eq!( + result, "openai:gpt-4o", + "empty coding_provider must be treated as unset and inherit chat BYOK" + ); + // agentic is excluded from BYOK inheritance regardless + config.agentic_provider = Some(String::new()); + let agentic_result = provider_for_role("agentic", &config); + assert_eq!( + agentic_result, "openhuman", + "empty agentic_provider must stay on managed backend even when chat BYOK is configured" + ); +} + +// ── resolve_byok_fallback_provider_string direct tests ─────────────────────── + +#[test] +fn resolve_byok_fallback_returns_none_when_no_byok() { + let config = Config::default(); + assert!( + resolve_byok_fallback_provider_string(&config).is_none(), + "all routes empty must return None" + ); +} + +#[test] +fn resolve_byok_fallback_returns_none_for_local_only() { + let mut config = Config::default(); + config.chat_provider = Some("ollama:llama3.1".to_string()); + config.reasoning_provider = Some("lmstudio:google/gemma".to_string()); + assert!( + resolve_byok_fallback_provider_string(&config).is_none(), + "only local providers must return None" + ); +} + +#[test] +fn resolve_byok_fallback_returns_some_for_openai() { + let mut config = Config::default(); + config.chat_provider = Some("openai:gpt-4o".to_string()); + let result = resolve_byok_fallback_provider_string(&config); + assert_eq!(result, Some("openai:gpt-4o".to_string())); +} + +#[test] +fn resolve_byok_fallback_returns_some_for_anthropic() { + let mut config = Config::default(); + config.reasoning_provider = Some("anthropic:claude-sonnet-4-6".to_string()); + let result = resolve_byok_fallback_provider_string(&config); + assert_eq!(result, Some("anthropic:claude-sonnet-4-6".to_string())); +} + +#[test] +fn resolve_byok_fallback_skips_empty_and_finds_next() { + let mut config = Config::default(); + config.chat_provider = Some(String::new()); // empty — skipped + config.reasoning_provider = Some("anthropic:claude-opus-4-7".to_string()); + let result = resolve_byok_fallback_provider_string(&config); + assert_eq!(result, Some("anthropic:claude-opus-4-7".to_string())); +} + +#[test] +fn byok_fallback_background_workloads_never_inherit() { + // Background workloads (memory, embeddings, heartbeat, learning, subconscious) + // must stay on the managed backend even when chat BYOK is configured. + let mut config = Config::default(); + config.cloud_providers.push(openai_entry("p_oai", "openai")); + config.chat_provider = Some("openai:gpt-4o".to_string()); + for role in &[ + "memory", + "embeddings", + "heartbeat", + "learning", + "subconscious", + ] { + let result = provider_for_role(role, &config); + assert_eq!( + result, "openhuman", + "background workload '{}' must not inherit chat BYOK", + role + ); + } +} + #[tokio::test] #[ignore = "requires live LM Studio on localhost:1234"] async fn live_lmstudio_provider_streams_thinking_and_text() {