diff --git a/app/scripts/e2e-run-all-flows.sh b/app/scripts/e2e-run-all-flows.sh index 9df79e6bb94..84b4a80295f 100755 --- a/app/scripts/e2e-run-all-flows.sh +++ b/app/scripts/e2e-run-all-flows.sh @@ -244,6 +244,7 @@ if should_run_suite "navigation"; then run "test/e2e/specs/navigation-settings-panels.spec.ts" "navigation-settings" "navigation" run "test/e2e/specs/command-palette.spec.ts" "command-palette" "navigation" run "test/e2e/specs/channels-smoke.spec.ts" "channels-smoke" "navigation" + run "test/e2e/specs/chip-tabs-keyboard.spec.ts" "chip-tabs-keyboard" "navigation" run "test/e2e/specs/guided-tour-gates.spec.ts" "guided-tour-gates" "navigation" _mini_summary "navigation" fi diff --git a/crates/openhuman-core/src/inference/provider/factory/routing.rs b/crates/openhuman-core/src/inference/provider/factory/routing.rs index 5af847d5c37..d8034b69e91 100644 --- a/crates/openhuman-core/src/inference/provider/factory/routing.rs +++ b/crates/openhuman-core/src/inference/provider/factory/routing.rs @@ -85,6 +85,22 @@ pub fn provider_for_role(role: &str, config: &Config) -> String { // route now stands alone and an unset one falls through to the managed // backend, the same as every other workload. + // #6938: the Chat UI model picker records its selection in + // `config.default_model` (the web-chat turn path stores the per-turn + // `model_override` there — see `build_session_agent`), but role + // resolution never consulted it, so a picked BYOK/local model still + // fell through to the managed backend and 401'd as "session expired". + // When the default names an explicit provider route, honour it here + // instead of the managed fallback. + if let Some(route) = default_model_route_for_role(role, config) { + log::info!( + "[providers][default-model-route] role={} route={}", + role, + route + ); + return route; + } + let resolved = resolve_primary_cloud_provider_string(config); // #5146 §2.1: the fallback itself is correct and stays — background @@ -113,6 +129,57 @@ pub fn provider_for_role(role: &str, config: &Config) -> String { } } +/// #6938: the provider route named by `config.default_model`, if it names an +/// explicit provider rather than a model for the managed backend. +/// +/// The Chat UI model picker (both the composer's per-turn pick, stored in +/// `default_model` by `build_session_agent`, and a hand-set `default_model` +/// in `config.toml`) records selections like `my-openai:gpt-4o` there, but +/// role resolution never consulted it — the turn fell through to the managed +/// backend regardless. Only the chat-tier roles consult the default: a +/// chat-model pick says nothing about which model should do vision or +/// summarization, so the background specialist roles keep their managed +/// fallback. +/// +/// Returns `None` (managed fallback unchanged) for everything that is not an +/// explicit route: tier hints (`hint:chat`), the AI settings row's managed +/// catalog pin (`openrouter//[:tag]` — a model name for the +/// managed backend, see `DefaultModelRow.tsx`), bare model ids, and slugs +/// that name no configured provider. The one bare provider name that is +/// honoured is `claude_agent_sdk`: unlike local runtimes and cloud slugs, +/// which need a `:model` part to construct, the SDK falls back to +/// `config.claude_agent_sdk.default_model` (see +/// `claude_agent_sdk_model_from_string`). +fn default_model_route_for_role(role: &str, config: &Config) -> Option { + match role { + "chat" | "reasoning" | "agentic" | "coding" | "burst" => {} + _ => return None, + } + let dm = config.default_model.as_deref()?.trim(); + if dm.is_empty() || dm.starts_with("hint:") || dm.starts_with("openrouter/") { + return None; + } + // Bare `claude_agent_sdk` is the only bare provider name honoured here: + // the SDK resolves its model from `config.claude_agent_sdk.default_model` + // (see `claude_agent_sdk_model_from_string`), while bare local runtimes + // (`ollama`, ...) and bare cloud slugs carry no `:model` part and fail + // at construction — they keep the managed fallback rather than erroring + // (tinysweeper review on #6996; CodeRabbit review on #6996). + if !dm.contains(':') { + return (dm == CLAUDE_AGENT_SDK_PROVIDER).then(|| dm.to_string()); + } + let (slug, rest) = dm.split_once(':')?; + let slug = slug.trim(); + if slug.is_empty() || slug == PROVIDER_OPENHUMAN || rest.trim().is_empty() { + return None; + } + let known = config.cloud_providers.iter().any(|e| e.slug == slug) + || tinyinference_local::profile::is_local_provider_string(dm) + || dm.starts_with(tinyagents_harness::providers::claude_code::PROVIDER_PREFIX) + || dm.starts_with(CLAUDE_AGENT_SDK_PREFIX); + known.then(|| dm.to_string()) +} + /// #3767: Whether the OpenHuman managed-credits gate should be bypassed for a /// single workload role. /// @@ -203,3 +270,7 @@ pub(super) fn split_model_and_temperature(raw: &str) -> (String, Option) { } (trimmed.to_string(), None) } + +#[cfg(test)] +#[path = "routing_tests.rs"] +mod routing_tests; diff --git a/crates/openhuman-core/src/inference/provider/factory/routing_tests.rs b/crates/openhuman-core/src/inference/provider/factory/routing_tests.rs new file mode 100644 index 00000000000..109b68a11d6 --- /dev/null +++ b/crates/openhuman-core/src/inference/provider/factory/routing_tests.rs @@ -0,0 +1,250 @@ +use super::*; +use crate::config::schema::cloud_providers::{AuthStyle, CloudProviderCreds}; +use crate::config::Config; +use crate::inference::provider::factory::resolves_to_managed_backend; + +fn byok_openai_compatible_entry() -> CloudProviderCreds { + CloudProviderCreds { + id: "p_my-openai".to_string(), + slug: "my-openai".to_string(), + label: "My OpenAI".to_string(), + endpoint: "https://inference.example.com/v1".to_string(), + auth_style: AuthStyle::Bearer, + ..Default::default() + } +} + +fn config_with_byok_provider() -> Config { + let mut config = Config::default(); + config.cloud_providers = vec![byok_openai_compatible_entry()]; + config +} + +/// #6938: the Chat UI model picker records its selection in +/// `config.default_model` (the web-chat turn path stores the per-turn +/// `model_override` there), but role resolution never consulted it — every +/// turn with an unset `chat_provider` silently fell back to the managed +/// backend and 401'd as "session expired" on local-only profiles. +#[test] +fn default_model_with_explicit_provider_routes_chat_instead_of_managed() { + let mut config = config_with_byok_provider(); + config.default_model = Some("my-openai:gpt-4o".to_string()); + + assert_eq!( + provider_for_role("chat", &config), + "my-openai:gpt-4o", + "a picked provider in default_model must route the chat turn" + ); + assert!( + !resolves_to_managed_backend("chat", &config), + "the chat turn must not resolve to the managed backend when a provider was picked" + ); +} + +/// The default-model route covers the chat-tier roles, whose turns the +/// picker is meant to influence. +#[test] +fn default_model_route_applies_to_chat_tier_roles() { + let mut config = config_with_byok_provider(); + config.default_model = Some("my-openai:gpt-4o".to_string()); + + for role in ["chat", "reasoning", "agentic", "coding"] { + assert_eq!( + provider_for_role(role, &config), + "my-openai:gpt-4o", + "`{role}` must honour the picked provider in default_model" + ); + } +} + +/// A chat-model pick says nothing about which model should do vision or +/// summarization — the background specialist roles keep their managed +/// fallback. +#[test] +fn default_model_route_does_not_touch_background_roles() { + let mut config = config_with_byok_provider(); + config.default_model = Some("my-openai:gpt-4o".to_string()); + + for role in [ + "vision", + "memory", + "summarization", + "embeddings", + "learning", + ] { + assert_eq!( + provider_for_role(role, &config), + "openhuman", + "`{role}` must keep the managed fallback despite the default_model pick" + ); + } +} + +/// Everything `default_model` can hold that is *not* an explicit provider +/// route must keep the managed fallback: tier hints, the AI settings row's +/// managed catalog pin (`openrouter/...`), bare model ids, unknown slugs, +/// and the managed provider itself. +#[test] +fn default_model_without_explicit_provider_keeps_managed_fallback() { + let mut config = config_with_byok_provider(); + for default_model in [ + "hint:chat", + "openrouter/deepseek/deepseek-v3", + "openrouter/deepseek/deepseek-v3:free", + "gpt-4o", + "", + "nosuchprovider:some-model", + "openhuman:some-model", + "my-openai:", + ":gpt-4o", + ] { + config.default_model = Some(default_model.to_string()); + assert_eq!( + provider_for_role("chat", &config), + "openhuman", + "default_model={default_model:?} must not reroute the chat turn" + ); + assert!( + resolves_to_managed_backend("chat", &config), + "default_model={default_model:?} must keep the managed backend" + ); + } +} + +/// A local runtime pick (`ollama:...`) routes the chat turn locally instead +/// of the managed backend — same mechanism, no cloud provider involved. +#[test] +fn default_model_with_local_provider_routes_chat_locally() { + let mut config = Config::default(); + config.default_model = Some("ollama:llama3".to_string()); + + assert_eq!( + provider_for_role("chat", &config), + "ollama:llama3", + "a picked local model must route the chat turn locally" + ); + assert!( + !resolves_to_managed_backend("chat", &config), + "a picked local model must not resolve to the managed backend" + ); +} + +/// An explicitly configured role route still wins over the default model — +/// the #6109 "each route stands alone" invariant is preserved. +#[test] +fn explicit_role_route_still_wins_over_default_model() { + let mut config = config_with_byok_provider(); + config.chat_provider = Some("anthropic:claude-x".to_string()); + config.default_model = Some("my-openai:gpt-4o".to_string()); + + assert_eq!( + provider_for_role("chat", &config), + "anthropic:claude-x", + "the explicitly configured chat_provider must win over default_model" + ); +} + +/// The #6109 regression in the other direction: a configured sibling route +/// is used for its own role while the default model covers the unset ones — +/// no cross-contamination either way. +#[test] +fn default_model_does_not_disturb_configured_sibling_routes() { + let mut config = config_with_byok_provider(); + config.coding_provider = Some("my-openai:codestral".to_string()); + config.default_model = Some("my-openai:gpt-4o".to_string()); + + assert_eq!( + provider_for_role("coding", &config), + "my-openai:codestral", + "the configured coding route must be honoured as-is" + ); + assert_eq!( + provider_for_role("chat", &config), + "my-openai:gpt-4o", + "the unset chat route falls back to the default_model pick, not the sibling's route" + ); + assert_eq!( + provider_for_role("reasoning", &config), + "my-openai:gpt-4o", + "the unset reasoning route falls back to the default_model pick, not the sibling's route" + ); +} + +/// tinysweeper review on #6996: a bare `claude_agent_sdk` default names the +/// provider with no `:model` part. The old `split_once(':')?` rejected it +/// before the explicit `CLAUDE_AGENT_SDK_PROVIDER` check could run, silently +/// falling back to the managed backend. +#[test] +fn default_model_with_bare_claude_agent_sdk_routes_chat() { + let mut config = Config::default(); + config.default_model = Some("claude_agent_sdk".to_string()); + + assert_eq!( + provider_for_role("chat", &config), + "claude_agent_sdk", + "a bare claude_agent_sdk pick must route the chat turn" + ); + assert!( + !resolves_to_managed_backend("chat", &config), + "a bare claude_agent_sdk pick must not resolve to the managed backend" + ); +} + +/// Bare local provider names (`ollama` with no `:model` part) keep the managed +/// fallback: the local runtime constructor rejects an empty model +/// (`empty_model_err` in `local_runtime.rs`), so routing there would only +/// trade the managed backend for a build-time error (CodeRabbit review on +/// #6996). +#[test] +fn default_model_with_bare_local_provider_keeps_managed_fallback() { + let mut config = Config::default(); + config.default_model = Some("ollama".to_string()); + + assert_eq!( + provider_for_role("chat", &config), + "openhuman", + "a bare ollama pick has no model id and must keep the managed fallback" + ); + assert!( + resolves_to_managed_backend("chat", &config), + "a bare ollama pick must resolve to the managed backend" + ); +} + +/// A bare cloud slug (`my-openai` with no `:model` part) is not constructible +/// either — the cloud-slug path requires the `:` form — so it +/// keeps the managed fallback as well. +#[test] +fn default_model_with_bare_cloud_slug_keeps_managed_fallback() { + let mut config = config_with_byok_provider(); + config.default_model = Some("my-openai".to_string()); + + assert_eq!( + provider_for_role("chat", &config), + "openhuman", + "a bare cloud slug has no model id and must keep the managed fallback" + ); + assert!( + resolves_to_managed_backend("chat", &config), + "a bare cloud slug must resolve to the managed backend" + ); +} + +/// Bare `openai` is deliberately NOT rerouted: the `openai` slug names the +/// cloud provider elsewhere in the factory, so it keeps the managed fallback +/// rather than being misread as a local runtime. +#[test] +fn default_model_with_bare_openai_keeps_managed_fallback() { + let mut config = Config::default(); + config.default_model = Some("openai".to_string()); + + assert_eq!( + provider_for_role("chat", &config), + "openhuman", + "bare openai must keep the managed fallback" + ); + assert!( + resolves_to_managed_backend("chat", &config), + "bare openai must resolve to the managed backend" + ); +} diff --git a/crates/openhuman-core/src/inference/provider/openhuman_backend_model_auth_tests.rs b/crates/openhuman-core/src/inference/provider/openhuman_backend_model_auth_tests.rs new file mode 100644 index 00000000000..e3d4332286a --- /dev/null +++ b/crates/openhuman-core/src/inference/provider/openhuman_backend_model_auth_tests.rs @@ -0,0 +1,46 @@ +use super::*; + +// #6932: the offline local profile is a valid sign-in with no TinyHumans +// account behind it. `classify_session_token` reports its `exp`-less token +// `Live`, so managed inference used to send it, collect a backend +// `401 "Invalid token"` and surface that as an expired session. +#[test] +fn the_offline_local_session_cannot_authenticate_managed_inference() { + use crate::security::credentials::session_support::{ + SessionTokenCheck, LOCAL_SESSION_MANAGED_INFERENCE_UNAVAILABLE, + }; + + let error = managed_bearer(SessionTokenCheck::Live("header.payload.local".to_string())) + .expect_err("a local session has no managed bearer"); + + assert_eq!( + error.to_string(), + LOCAL_SESSION_MANAGED_INFERENCE_UNAVAILABLE + ); +} + +#[test] +fn the_refusal_does_not_read_as_an_expired_session() { + use crate::security::credentials::session_support::SessionTokenCheck; + + let error = managed_bearer(SessionTokenCheck::Live("header.payload.local".to_string())) + .expect_err("a local session has no managed bearer"); + + // The whole point of the fix: this must not reach the sign-out path that + // the backend's 401 envelope used to trigger. + assert!(!crate::core::observability::is_session_expired_message( + &error.to_string() + )); +} + +#[test] +fn a_signed_in_session_still_authenticates_managed_inference() { + use crate::security::credentials::session_support::SessionTokenCheck; + + let bearer = managed_bearer(SessionTokenCheck::Live( + "header.payload.signature".to_string(), + )) + .expect("a hosted session is the bearer"); + + assert_eq!(bearer, "header.payload.signature"); +} diff --git a/crates/openhuman-core/src/inference/provider/openhuman_backend_model_tests.rs b/crates/openhuman-core/src/inference/provider/openhuman_backend_model_tests.rs index 7b6a30fdba4..dcfdc5e1a83 100644 --- a/crates/openhuman-core/src/inference/provider/openhuman_backend_model_tests.rs +++ b/crates/openhuman-core/src/inference/provider/openhuman_backend_model_tests.rs @@ -711,50 +711,7 @@ fn resolve_bearer_returns_token_for_exp_less_offline_session() { .expect("an exp-less offline session must resolve (presence-only)"); assert_eq!(token, "test.session.jwt"); } +#[path = "openhuman_backend_model_auth_tests.rs"] +mod auth_tests; #[path = "openhuman_backend_model_endpoint_tests.rs"] mod endpoint_tests; - -// #6932: the offline local profile is a valid sign-in with no TinyHumans -// account behind it. `classify_session_token` reports its `exp`-less token -// `Live`, so managed inference used to send it, collect a backend -// `401 "Invalid token"` and surface that as an expired session. -#[test] -fn the_offline_local_session_cannot_authenticate_managed_inference() { - use crate::security::credentials::session_support::{ - SessionTokenCheck, LOCAL_SESSION_MANAGED_INFERENCE_UNAVAILABLE, - }; - - let error = managed_bearer(SessionTokenCheck::Live("header.payload.local".to_string())) - .expect_err("a local session has no managed bearer"); - - assert_eq!( - error.to_string(), - LOCAL_SESSION_MANAGED_INFERENCE_UNAVAILABLE - ); -} - -#[test] -fn the_refusal_does_not_read_as_an_expired_session() { - use crate::security::credentials::session_support::SessionTokenCheck; - - let error = managed_bearer(SessionTokenCheck::Live("header.payload.local".to_string())) - .expect_err("a local session has no managed bearer"); - - // The whole point of the fix: this must not reach the sign-out path that - // the backend's 401 envelope used to trigger. - assert!(!crate::core::observability::is_session_expired_message( - &error.to_string() - )); -} - -#[test] -fn a_signed_in_session_still_authenticates_managed_inference() { - use crate::security::credentials::session_support::SessionTokenCheck; - - let bearer = managed_bearer(SessionTokenCheck::Live( - "header.payload.signature".to_string(), - )) - .expect("a hosted session is the bearer"); - - assert_eq!(bearer, "header.payload.signature"); -} diff --git a/tests/json_rpc_e2e.rs b/tests/json_rpc_e2e.rs index c902c90affa..ac303d501c5 100644 --- a/tests/json_rpc_e2e.rs +++ b/tests/json_rpc_e2e.rs @@ -4006,7 +4006,7 @@ async fn json_rpc_web_chat_custom_chat_provider_uses_stored_key_and_rebuilds_on_ "endpoint": mock_origin, "auth_style": "bearer" }], - "chat_provider": "openai:gpt-4.1-mini" + "default_model": "openai:gpt-4.1-mini" }), ) .await; @@ -4089,7 +4089,7 @@ async fn json_rpc_web_chat_custom_chat_provider_uses_stored_key_and_rebuilds_on_ 6005, "openhuman.update_model_settings", json!({ - "chat_provider": "openai:gpt-4.1-nano" + "default_model": "openai:gpt-4.1-nano" }), ) .await;