From 9dd792c8b80caa88822476430975f2fc38388ade Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sun, 19 Apr 2026 11:43:38 -0400 Subject: [PATCH] fix(install): exclude openai-compatible from v1 setup Restrict the browser install flow to Anthropic, OpenAI, and Gemini, remove the unused install-time base URL surface, and reject openai_compatible with a stable 422 response. Also fix the finishing health poller so it only redirects after the server comes back healthy outside install mode instead of jumping early on transient restart failures. --- apps/fabro-web/app/install-api.ts | 2 - apps/fabro-web/app/install-app.tsx | 93 +++++-------------- apps/fabro-web/app/install-config.test.ts | 13 +++ apps/fabro-web/app/install-config.ts | 17 ++++ apps/fabro-web/app/install-flow.test.ts | 38 ++++++++ apps/fabro-web/app/install-flow.ts | 9 ++ docs/api-reference/fabro-api.yaml | 14 +-- lib/crates/fabro-server/src/install.rs | 82 +++++----------- .../tests/it/api/install_openai_compatible.rs | 54 +++++++++++ lib/crates/fabro-server/tests/it/api/mod.rs | 1 + 10 files changed, 183 insertions(+), 140 deletions(-) create mode 100644 apps/fabro-web/app/install-config.test.ts create mode 100644 apps/fabro-web/app/install-config.ts create mode 100644 apps/fabro-web/app/install-flow.test.ts create mode 100644 apps/fabro-web/app/install-flow.ts create mode 100644 lib/crates/fabro-server/tests/it/api/install_openai_compatible.rs diff --git a/apps/fabro-web/app/install-api.ts b/apps/fabro-web/app/install-api.ts index 7ee45540e..4875fd444 100644 --- a/apps/fabro-web/app/install-api.ts +++ b/apps/fabro-web/app/install-api.ts @@ -5,7 +5,6 @@ export interface InstallSessionResponse { providers: Array<{ provider: string; configured: boolean; - openai_base_url?: string | null; }>; } | null; @@ -32,7 +31,6 @@ export interface InstallFinishResponse { export interface InstallLlmProviderInput { provider: string; api_key: string; - openai_base_url?: string | null; } export interface InstallGithubAppManifestInput { diff --git a/apps/fabro-web/app/install-app.tsx b/apps/fabro-web/app/install-app.tsx index 6e49949e5..afb620964 100644 --- a/apps/fabro-web/app/install-app.tsx +++ b/apps/fabro-web/app/install-app.tsx @@ -19,6 +19,8 @@ import { testInstallLlm, } from "./install-api"; import { AuthLayout } from "./components/auth-layout"; +import { INSTALL_PROVIDERS } from "./install-config"; +import { shouldRedirectAfterHealthPoll } from "./install-flow"; import { consumeInstallTokenFromUrl } from "./mode"; const INSTALL_STEPS = [ @@ -29,29 +31,6 @@ const INSTALL_STEPS = [ { id: "review", label: "Review", href: "/install/review" }, ] as const; -const PROVIDERS = [ - { - id: "anthropic", - label: "Anthropic", - hint: "Claude API key.", - }, - { - id: "openai", - label: "OpenAI", - hint: "Responses API key.", - }, - { - id: "gemini", - label: "Gemini", - hint: "Google AI Studio API key.", - }, - { - id: "openai_compatible", - label: "OpenAI Compatible", - hint: "API key plus a custom base URL.", - }, -] as const; - type StepId = (typeof INSTALL_STEPS)[number]["id"]; type FinishState = InstallFinishResponse | null; type GithubStrategy = "token" | "app"; @@ -61,7 +40,6 @@ type ProviderSelection = Record< string, { apiKey: string; - openaiBaseUrl: string; } >; @@ -175,16 +153,22 @@ export default function InstallApp() { const interval = window.setInterval(async () => { try { const response = await fetch("/health"); - if (!response.ok) { - window.location.href = finishState.restart_url; - return; - } - const body = (await response.json()) as { mode?: string }; - if (body.mode !== "install") { + const body = response.ok + ? ((await response.json()) as { mode?: string }) + : undefined; + if ( + shouldRedirectAfterHealthPoll({ + kind: "response", + ok: response.ok, + mode: body?.mode, + }) + ) { window.location.href = finishState.restart_url; } } catch { - window.location.href = finishState.restart_url; + if (shouldRedirectAfterHealthPoll({ kind: "error" })) { + window.location.href = finishState.restart_url; + } } }, 1_000); @@ -265,12 +249,11 @@ export default function InstallApp() { error={saveError} submitting={submitting} onSubmit={async () => { - const providers = PROVIDERS.map(({ id }) => { - const current = llmSelection[id] ?? { apiKey: "", openaiBaseUrl: "" }; + const providers = INSTALL_PROVIDERS.map(({ id }) => { + const current = llmSelection[id] ?? { apiKey: "" }; return { provider: id, api_key: current.apiKey.trim(), - openai_base_url: current.openaiBaseUrl.trim() || null, }; }).filter((provider) => provider.api_key.length > 0); @@ -744,13 +727,8 @@ function ReviewScreen({ - provider.openai_base_url - ? `${provider.provider} (${provider.openai_base_url})` - : provider.provider, - ) - .join(", ") || "Not configured" + (session?.llm?.providers ?? []).map((provider) => provider.provider).join(", ") || + "Not configured" } /> void; }) { return ( -
- {PROVIDERS.map((provider) => { - const current = value[provider.id] ?? { apiKey: "", openaiBaseUrl: "" }; +
+ {INSTALL_PROVIDERS.map((provider) => { + const current = value[provider.id] ?? { apiKey: "" }; return (
- {provider.id === "openai_compatible" ? ( -
- - - onChange({ - ...value, - [provider.id]: { - ...current, - openaiBaseUrl: event.target.value, - }, - })} - className={INPUT_CLASS} - placeholder="https://api.example.com/v1" - /> - -
- ) : null}
); })} @@ -1054,11 +1013,10 @@ function Field({ function defaultProviderSelection(): ProviderSelection { return Object.fromEntries( - PROVIDERS.map((provider) => [ + INSTALL_PROVIDERS.map((provider) => [ provider.id, { apiKey: "", - openaiBaseUrl: "", }, ]), ); @@ -1068,16 +1026,13 @@ function hydrateProviderSelection( current: ProviderSelection, session: InstallSessionResponse, ): ProviderSelection { - const hasUserInput = Object.values(current).some( - (provider) => provider.apiKey || provider.openaiBaseUrl, - ); + const hasUserInput = Object.values(current).some((provider) => provider.apiKey); if (hasUserInput) return current; const next = defaultProviderSelection(); for (const provider of session.llm?.providers ?? []) { next[provider.provider] = { apiKey: "", - openaiBaseUrl: provider.openai_base_url ?? "", }; } return next; diff --git a/apps/fabro-web/app/install-config.test.ts b/apps/fabro-web/app/install-config.test.ts new file mode 100644 index 000000000..757ee6a1e --- /dev/null +++ b/apps/fabro-web/app/install-config.test.ts @@ -0,0 +1,13 @@ +import { describe, expect, test } from "bun:test"; + +import { INSTALL_PROVIDERS } from "./install-config"; + +describe("INSTALL_PROVIDERS", () => { + test("excludes openai_compatible from install v1", () => { + expect(INSTALL_PROVIDERS.map((provider) => provider.id)).toEqual([ + "anthropic", + "openai", + "gemini", + ]); + }); +}); diff --git a/apps/fabro-web/app/install-config.ts b/apps/fabro-web/app/install-config.ts new file mode 100644 index 000000000..101251ba9 --- /dev/null +++ b/apps/fabro-web/app/install-config.ts @@ -0,0 +1,17 @@ +export const INSTALL_PROVIDERS = [ + { + id: "anthropic", + label: "Anthropic", + hint: "Claude API key.", + }, + { + id: "openai", + label: "OpenAI", + hint: "Responses API key.", + }, + { + id: "gemini", + label: "Gemini", + hint: "Google AI Studio API key.", + }, +] as const; diff --git a/apps/fabro-web/app/install-flow.test.ts b/apps/fabro-web/app/install-flow.test.ts new file mode 100644 index 000000000..900cdb23c --- /dev/null +++ b/apps/fabro-web/app/install-flow.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, test } from "bun:test"; + +import { shouldRedirectAfterHealthPoll } from "./install-flow"; + +describe("shouldRedirectAfterHealthPoll", () => { + test("waits when the health request fails", () => { + expect(shouldRedirectAfterHealthPoll({ kind: "error" })).toBe(false); + }); + + test("waits when the server returns a non-success status", () => { + expect( + shouldRedirectAfterHealthPoll({ + kind: "response", + ok: false, + }), + ).toBe(false); + }); + + test("waits while the server is still in install mode", () => { + expect( + shouldRedirectAfterHealthPoll({ + kind: "response", + ok: true, + mode: "install", + }), + ).toBe(false); + }); + + test("redirects only after the server returns success outside install mode", () => { + expect( + shouldRedirectAfterHealthPoll({ + kind: "response", + ok: true, + mode: "normal", + }), + ).toBe(true); + }); +}); diff --git a/apps/fabro-web/app/install-flow.ts b/apps/fabro-web/app/install-flow.ts new file mode 100644 index 000000000..c3104a21d --- /dev/null +++ b/apps/fabro-web/app/install-flow.ts @@ -0,0 +1,9 @@ +export type FinishHealthPollResult = + | { kind: "error" } + | { kind: "response"; ok: boolean; mode?: string }; + +export function shouldRedirectAfterHealthPoll( + result: FinishHealthPollResult, +): boolean { + return result.kind === "response" && result.ok && result.mode !== "install"; +} diff --git a/docs/api-reference/fabro-api.yaml b/docs/api-reference/fabro-api.yaml index 095ba16ac..bc59814eb 100644 --- a/docs/api-reference/fabro-api.yaml +++ b/docs/api-reference/fabro-api.yaml @@ -2007,7 +2007,7 @@ components: example: true InstallLlmTestInput: - description: Input for install-time LLM credential validation. + description: Input for install-time LLM credential validation. Supported providers in install v1 are `anthropic`, `openai`, and `gemini`. type: object required: - provider @@ -2018,10 +2018,6 @@ components: example: anthropic api_key: type: string - openai_base_url: - type: string - format: uri - description: Optional override base URL used for OpenAI-style providers during install. InstallLlmProvidersInput: description: LLM providers selected during browser install. @@ -2036,7 +2032,7 @@ components: $ref: "#/components/schemas/InstallLlmProviderInput" InstallLlmProviderInput: - description: One persisted LLM provider configuration collected during browser install. + description: One persisted LLM provider configuration collected during browser install. Supported providers in install v1 are `anthropic`, `openai`, and `gemini`. type: object required: - provider @@ -2047,9 +2043,6 @@ components: example: anthropic api_key: type: string - openai_base_url: - type: string - format: uri InstallLlmSummary: description: Redacted summary of persisted LLM install choices. @@ -2067,9 +2060,6 @@ components: type: string configured: type: boolean - openai_base_url: - type: string - format: uri InstallServerConfigInput: description: Canonical server URL confirmed during browser install. diff --git a/lib/crates/fabro-server/src/install.rs b/lib/crates/fabro-server/src/install.rs index c1f76440b..3e1d04b4c 100644 --- a/lib/crates/fabro-server/src/install.rs +++ b/lib/crates/fabro-server/src/install.rs @@ -128,10 +128,8 @@ struct LlmProvidersInput { #[derive(Clone, Debug, Deserialize, Serialize)] struct LlmProviderInput { - provider: Provider, - api_key: String, - #[serde(default)] - openai_base_url: Option, + provider: Provider, + api_key: String, } #[derive(Clone, Debug, Deserialize, Serialize)] @@ -175,10 +173,8 @@ struct GithubAppInstall { #[derive(Clone, Debug, Deserialize)] struct InstallLlmTestInput { - provider: Provider, - api_key: String, - #[serde(default)] - openai_base_url: Option, + provider: Provider, + api_key: String, } #[derive(Clone, Debug, Deserialize)] @@ -374,21 +370,13 @@ async fn post_install_llm_test( return response; } + if let Some(error) = unsupported_install_provider_error(input.provider) { + return install_error_response(StatusCode::UNPROCESSABLE_ENTITY, error); + } + if input.api_key.trim().is_empty() { return install_error_response(StatusCode::UNPROCESSABLE_ENTITY, "api_key is required"); } - if input.provider == Provider::OpenAiCompatible - && input - .openai_base_url - .as_deref() - .is_none_or(|value| value.trim().is_empty()) - { - return install_error_response( - StatusCode::UNPROCESSABLE_ENTITY, - "openai_base_url is required for openai_compatible", - ); - } - match validate_llm_provider(&state, &input).await { Ok(()) => Json(serde_json::json!({ "ok": true })).into_response(), Err(err) => { @@ -417,23 +405,15 @@ async fn put_install_llm( } for provider in &input.providers { + if let Some(error) = unsupported_install_provider_error(provider.provider) { + return install_error_response(StatusCode::UNPROCESSABLE_ENTITY, error); + } if provider.api_key.trim().is_empty() { return install_error_response( StatusCode::UNPROCESSABLE_ENTITY, format!("api_key is required for {}", provider.provider.as_str()), ); } - if provider.provider == Provider::OpenAiCompatible - && provider - .openai_base_url - .as_deref() - .is_none_or(|value| value.trim().is_empty()) - { - return install_error_response( - StatusCode::UNPROCESSABLE_ENTITY, - "openai_base_url is required for openai_compatible", - ); - } } state @@ -445,6 +425,13 @@ async fn put_install_llm( StatusCode::NO_CONTENT.into_response() } +fn unsupported_install_provider_error(provider: Provider) -> Option<&'static str> { + match provider { + Provider::OpenAiCompatible => Some("openai_compatible is not supported by install in v1"), + _ => None, + } +} + async fn put_install_server( State(state): State, headers: HeaderMap, @@ -710,22 +697,6 @@ async fn post_install_finish( secret_type: VaultSecretType::Credential, description: None, }); - if let Some(base_url) = provider - .openai_base_url - .as_deref() - .filter(|value| !value.trim().is_empty()) - { - vault_secrets.push(VaultSecretWrite { - name: if provider.provider == Provider::OpenAiCompatible { - "OPENAI_COMPATIBLE_BASE_URL".to_string() - } else { - "OPENAI_BASE_URL".to_string() - }, - value: base_url.to_string(), - secret_type: VaultSecretType::Environment, - description: None, - }); - } } let mut server_env_secrets = Vec::new(); @@ -929,7 +900,6 @@ fn redacted_llm(pending_install: &PendingInstall) -> serde_json::Value { "providers": llm.providers.iter().map(|provider| serde_json::json!({ "provider": provider.provider.as_str(), "configured": true, - "openai_base_url": provider.openai_base_url, })).collect::>() }) }, @@ -1039,7 +1009,7 @@ async fn validate_llm_provider( provider: input.provider, auth_header, extra_headers: HashMap::new(), - base_url: provider_base_url(state, input.provider, input.openai_base_url.as_deref()), + base_url: provider_base_url(state, input.provider), codex_mode: false, org_id: None, project_id: None, @@ -1067,14 +1037,12 @@ async fn validate_llm_provider( .map_err(|err| err.to_string()) } -fn provider_base_url( - state: &InstallAppState, - provider: Provider, - override_url: Option<&str>, -) -> Option { - override_url - .map(ToString::to_string) - .or_else(|| state.upstreams.provider_base_urls.get(&provider).cloned()) +fn provider_base_url(state: &InstallAppState, provider: Provider) -> Option { + state + .upstreams + .provider_base_urls + .get(&provider) + .cloned() .or_else(|| match provider { Provider::Anthropic => std::env::var("ANTHROPIC_BASE_URL").ok(), Provider::OpenAi => std::env::var("OPENAI_BASE_URL").ok(), diff --git a/lib/crates/fabro-server/tests/it/api/install_openai_compatible.rs b/lib/crates/fabro-server/tests/it/api/install_openai_compatible.rs new file mode 100644 index 000000000..6b3f5b67b --- /dev/null +++ b/lib/crates/fabro-server/tests/it/api/install_openai_compatible.rs @@ -0,0 +1,54 @@ +use axum::body::Body; +use axum::http::{Request, StatusCode}; +use fabro_server::install::{InstallAppState, build_install_router}; +use tower::ServiceExt; + +use crate::helpers::body_json; + +#[tokio::test] +async fn install_llm_endpoints_reject_openai_compatible_in_v1() { + let app = build_install_router(InstallAppState::for_test("test-install-token")); + + let test_response = app + .clone() + .oneshot( + Request::builder() + .method("POST") + .uri("/install/llm/test") + .header("authorization", "Bearer test-install-token") + .header("content-type", "application/json") + .body(Body::from( + r#"{"provider":"openai_compatible","api_key":"test-key"}"#, + )) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(test_response.status(), StatusCode::UNPROCESSABLE_ENTITY); + let test_body = body_json(test_response.into_body()).await; + assert_eq!( + test_body["error"], + "openai_compatible is not supported by install in v1" + ); + + let put_response = app + .oneshot( + Request::builder() + .method("PUT") + .uri("/install/llm") + .header("authorization", "Bearer test-install-token") + .header("content-type", "application/json") + .body(Body::from( + r#"{"providers":[{"provider":"openai_compatible","api_key":"test-key"}]}"#, + )) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(put_response.status(), StatusCode::UNPROCESSABLE_ENTITY); + let put_body = body_json(put_response.into_body()).await; + assert_eq!( + put_body["error"], + "openai_compatible is not supported by install in v1" + ); +} diff --git a/lib/crates/fabro-server/tests/it/api/mod.rs b/lib/crates/fabro-server/tests/it/api/mod.rs index 39275ad12..e68d350a7 100644 --- a/lib/crates/fabro-server/tests/it/api/mod.rs +++ b/lib/crates/fabro-server/tests/it/api/mod.rs @@ -1,4 +1,5 @@ mod install; +mod install_openai_compatible; mod routing; mod runs; mod settings;