Merge pull request #1055 from fabro-sh/fix/no-bare-selector-passthrough

Reject bare provider names and unknown unpinned model selectors

docs/public/api-reference/fabro-api.yaml

@@ -9012,12 +9012,16 @@ components:
         model:
           type: string
           description: >-
-            Model selector: `provider/model`, a model id or alias, or a
-            provider id. The server picks a ready-provider default when
-            omitted.
+            Model selector: `provider/model`, or a model id or alias. A bare
+            provider name is rejected; pin it with `provider` instead. When
+            omitted, the server picks the pinned provider's default model, or
+            a ready-provider default when `provider` is also omitted.
         provider:
           type: string
-          description: Optional provider pin for a bare model selector.
+          description: >-
+            Optional provider pin. Scopes a bare model selector to that
+            provider, which also lets an uncatalogued model id reach a
+            provider that allows passthrough.
         system:
           type: string
           description: System prompt (convenience; prepended as a system message).

docs/public/core-concepts/models.mdx

@@ -130,7 +130,7 @@ capabilities = { text = true, tools = true }
 `api_model` is the model name sent to that provider's API. It defaults to the exact model id, so omit it when the two strings match. Fabro does not infer vendor prefixes or rewrite the value.
 
 <Note>
-A `provider/model` selector such as `openai/gpt-5.6-sol` pins the provider and names the model by id, alias, or wire id. A bare selector with no provider pin picks the highest-priority ready offering; a separate `provider = "openrouter"` pin selects the OpenRouter offering. Providers with `allow_passthrough = true` also accept `provider/model` selectors for models the catalog does not list.
+A `provider/model` selector such as `openai/gpt-5.6-sol` pins the provider and names the model by id, alias, or wire id. A bare selector with no provider pin picks the highest-priority ready offering; a separate `provider = "openrouter"` pin selects the OpenRouter offering. Providers with `allow_passthrough = true` also accept `provider/model` selectors for models the catalog does not list. A bare selector the catalog does not know is an error rather than a passthrough, and a bare provider name such as `gemini` is an error too: pin the provider and omit the model to use its default.
 </Note>
 
 Model roles are separate: the provider's `default_model` controls normal model selection for workflow execution, while `small_default = true` on a model row marks the provider's small utility model for metadata tasks such as generated run titles. If a provider has no small default, Fabro falls back to that provider's default model.

lib/apps/fabro-server/src/server/handler/models.rs

@@ -299,7 +299,8 @@ fn model_selection_response(error: &ModelSelectionError) -> Response {
     match error {
         ModelSelectionError::UnknownProvider { .. }
         | ModelSelectionError::UnknownSelector { .. }
-        | ModelSelectionError::UnknownSelectorOnProvider { .. } => {
+        | ModelSelectionError::UnknownSelectorOnProvider { .. }
+        | ModelSelectionError::ProviderUsedAsModel { .. } => {
             ApiError::not_found(error.to_string()).into_response()
         }
         ModelSelectionError::ProviderUnavailable { .. }

lib/apps/fabro-server/src/server/handler/sessions.rs

@@ -1523,10 +1523,8 @@ enabled = true
                 .unwrap(),
             (openrouter, "future-model".to_string())
         );
-        assert_eq!(
-            canonical_session_model(&catalog, &both, Some("future-model"), None).unwrap(),
-            (openai, "future-model".to_string())
-        );
+        // Without a pin there is no provider to pass through on.
+        assert!(canonical_session_model(&catalog, &both, Some("future-model"), None).is_err());
     }
 
     /// A colon in a model ID does not make it provider-qualified. Ollama
@@ -1548,9 +1546,8 @@ enabled = true
             .unwrap(),
             (openrouter, "future-model:latest".to_string())
         );
-        assert_eq!(
-            canonical_session_model(&catalog, &both, Some("future-model:latest"), None).unwrap(),
-            (openai, "future-model:latest".to_string())
+        assert!(
+            canonical_session_model(&catalog, &both, Some("future-model:latest"), None).is_err()
         );
     }
 

lib/apps/fabro-server/tests/it/api/sessions.rs

@@ -222,11 +222,10 @@ async fn unknown_session_models_preserve_passthrough_on_the_selected_provider()
     let app = fabro_server::test_support::build_test_router(test_app_state());
     let (run_id, _run_id_workspace) = create_run(&app).await;
 
-    for model in ["not-a-real-model", "openai/not-a-real-model"] {
-        let created = create_session_with_model(&app, &run_id, "Ask Fabro", model).await;
-        assert_eq!(created["model"], "not-a-real-model");
-        assert_eq!(created["provider"], "openai");
-    }
+    let created =
+        create_session_with_model(&app, &run_id, "Ask Fabro", "openai/not-a-real-model").await;
+    assert_eq!(created["model"], "not-a-real-model");
+    assert_eq!(created["provider"], "openai");
 }
 
 #[tokio::test]
@@ -234,7 +233,7 @@ async fn invalid_session_model_refs_are_rejected_at_creation() {
     let app = fabro_server::test_support::build_test_router(test_app_state());
     let (run_id, _run_id_workspace) = create_run(&app).await;
 
-    for model in ["openai", "openai/", "anthropic/gpt-5.4"] {
+    for model in ["openai", "openai/", "anthropic/gpt-5.4", "not-a-real-model"] {
         let response = create_session_response(
             &app,
             &run_id,

lib/apps/fabro-server/tests/it/scenario/dry_run.rs

@@ -167,6 +167,48 @@ async fn completion_non_streaming_returns_valid_json() {
     assert!(body["usage"]["output"].is_number());
 }
 
+/// A provider name in the model field, or a selector the catalog does not
+/// know, is refused before any provider is called. Neither may pass through
+/// to whichever ready provider wins priority.
+#[tokio::test]
+async fn completion_with_a_bare_provider_or_unknown_selector_is_rejected_before_any_call() {
+    let mock_server = MockServer::start_async().await;
+    let anthropic = mock_server
+        .mock_async(|when, then| {
+            when.method("POST").path("/v1/messages");
+            then.status(500);
+        })
+        .await;
+    let app = test_app_with_mock_anthropic(&format!("{}/v1", mock_server.url("")));
+
+    for (model, expected_detail) in [
+        ("gemini", "set provider = \"gemini\""),
+        ("anthropic", "set provider = \"anthropic\""),
+        (
+            "not-a-real-model",
+            "unknown model selector 'not-a-real-model'",
+        ),
+    ] {
+        let response = app
+            .clone()
+            .oneshot(completion_request_with_model(false, model))
+            .await
+            .unwrap();
+        let body = response_json(
+            response,
+            StatusCode::BAD_REQUEST,
+            &format!("POST /api/v1/completions with model {model}"),
+        )
+        .await;
+        let detail = body["errors"][0]["detail"].as_str().unwrap_or_default();
+        assert!(
+            detail.contains(expected_detail),
+            "model {model}: unexpected detail {detail:?}"
+        );
+    }
+    assert_eq!(anthropic.calls_async().await, 0);
+}
+
 #[tokio::test]
 async fn completion_streaming_returns_sse() {
     let mock_server = MockServer::start_async().await;

lib/components/fabro-llm/src/lib.rs

@@ -5,9 +5,10 @@
 //!
 //! - building the catalog from the lithos built-ins and the operator `[llm]`
 //!   overlay, and reading the agent harness a model expects ([`catalog`]);
-//! - Fabro's passthrough policy for selections made before a request exists
-//!   ([`selection`]); at request time the lithos resolver enforces `enabled`
-//!   and `stands_in_for` itself;
+//! - Fabro's selection policy for choices made before a request exists
+//!   ([`selection`]), including when an uncatalogued model may pass through; at
+//!   request time the lithos resolver enforces `enabled` and `stands_in_for`
+//!   itself;
 //! - constructing a client from a Fabro credential store ([`client`]);
 //! - model and provider probes ([`probe`]), and the API views of the catalog
 //!   ([`api`]);

lib/components/fabro-llm/src/selection.rs

@@ -9,7 +9,10 @@
 //! - `provider/model` pins the provider, as the lithos resolver reads it.
 //! - An unknown selector pinned to a provider passes through verbatim on that
 //!   provider.
-//! - An unqualified unknown selector passes through on the default provider.
+//! - An unqualified unknown selector is an error. Passthrough needs a provider
+//!   pin, so a typo never lands on whichever provider happens to win priority.
+//! - A bare provider name is an error that points at the provider pin; it is
+//!   never a model name.
 //! - No selector picks the default offering (of the pinned provider, when one
 //!   is given).
 //!
@@ -73,6 +76,14 @@ pub enum ModelSelectionError {
         selector: String,
         provider: ProviderId,
     },
+    #[error(
+        "'{selector}' names the provider '{provider}', not a model; set provider = \"{provider}\" \
+         and omit the model to use its default"
+    )]
+    ProviderUsedAsModel {
+        selector: String,
+        provider: ProviderId,
+    },
     #[error(
         "model selector '{selector}' is known but has no offering on an eligible provider; available providers: {providers:?}"
     )]
@@ -155,6 +166,14 @@ pub fn select<'a>(
     }
     let matches = catalog.offerings_matching(selector);
     if matches.is_empty() {
+        // A provider name in the model position is the one unknown selector
+        // with an obvious intent, so say what the caller meant to write.
+        if let Some(provider) = catalog.enabled_provider(selector) {
+            return Err(ModelSelectionError::ProviderUsedAsModel {
+                selector: selector.to_string(),
+                provider: provider.id().clone(),
+            });
+        }
         return Err(ModelSelectionError::UnknownSelector {
             selector: selector.to_string(),
         });
@@ -230,13 +249,6 @@ pub fn resolve_selection(
                 model: selector,
             })
         }
-        Err(ModelSelectionError::UnknownSelector { .. }) => {
-            let default = select_default(catalog, eligible)?;
-            Ok(SelectedModel {
-                provider: default.provider.id().clone(),
-                model:    selector.to_string(),
-            })
-        }
         Err(error) => Err(error),
     }
 }
@@ -296,17 +308,53 @@ mod tests {
     }
 
     #[test]
-    fn unknown_selector_passes_through_on_the_default_provider() {
+    fn unknown_unqualified_selector_is_an_error_not_a_passthrough() {
         let catalog = test_catalog();
-        let selected = resolve_selection(
+        let error = resolve_selection(
             &catalog,
             Some("totally-new-model"),
             None,
             &eligible(&["openai", "anthropic"]),
         )
+        .unwrap_err();
+        assert_eq!(error, ModelSelectionError::UnknownSelector {
+            selector: "totally-new-model".to_string(),
+        });
+    }
+
+    #[test]
+    fn bare_provider_name_is_rejected_with_a_pin_hint() {
+        let catalog = test_catalog();
+        for (selector, provider) in [
+            ("openai", builtin::openai()),
+            ("anthropic", builtin::anthropic()),
+        ] {
+            let error = resolve_selection(
+                &catalog,
+                Some(selector),
+                None,
+                &eligible(&["openai", "anthropic"]),
+            )
+            .unwrap_err();
+            assert_eq!(error, ModelSelectionError::ProviderUsedAsModel {
+                selector: selector.to_string(),
+                provider: provider.clone(),
+            });
+            assert!(
+                error
+                    .to_string()
+                    .contains(&format!("provider = \"{provider}\""))
+            );
+        }
+        // The pin alone still means that provider's default.
+        let selected = resolve_selection(
+            &catalog,
+            None,
+            Some(&builtin::openai()),
+            &eligible(&["openai"]),
+        )
         .unwrap();
-        assert_eq!(selected.provider, builtin::anthropic());
-        assert_eq!(selected.model, "totally-new-model");
+        assert_eq!(selected.provider, builtin::openai());
     }
 
     #[test]

lib/packages/fabro-api-client/src/models/create-completion-request.ts

@@ -41,11 +41,11 @@ export interface CreateCompletionRequest {
      */
     'messages': Array<CompletionMessage>;
     /**
-     * Model selector: `provider/model`, a model id or alias, or a provider id. The server picks a ready-provider default when omitted.
+     * Model selector: `provider/model`, or a model id or alias. A bare provider name is rejected; pin it with `provider` instead. When omitted, the server picks the pinned provider\'s default model, or a ready-provider default when `provider` is also omitted.
      */
     'model'?: string;
     /**
-     * Optional provider pin for a bare model selector.
+     * Optional provider pin. Scopes a bare model selector to that provider, which also lets an uncatalogued model id reach a provider that allows passthrough.
      */
     'provider'?: string;
     /**