mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-10 22:43:37 +00:00
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.
This commit is contained in:
parent
ecdfdd82d8
commit
9dd792c8b8
10 changed files with 183 additions and 140 deletions
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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({
|
|||
<SummaryCard
|
||||
title="LLM"
|
||||
body={
|
||||
(session?.llm?.providers ?? [])
|
||||
.map((provider) =>
|
||||
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"
|
||||
}
|
||||
/>
|
||||
<SummaryCard
|
||||
|
|
@ -839,9 +817,9 @@ function ProviderFields({
|
|||
onChange: (nextValue: ProviderSelection) => void;
|
||||
}) {
|
||||
return (
|
||||
<div className="space-y-4">
|
||||
{PROVIDERS.map((provider) => {
|
||||
const current = value[provider.id] ?? { apiKey: "", openaiBaseUrl: "" };
|
||||
<div className="space-y-4">
|
||||
{INSTALL_PROVIDERS.map((provider) => {
|
||||
const current = value[provider.id] ?? { apiKey: "" };
|
||||
return (
|
||||
<div
|
||||
key={provider.id}
|
||||
|
|
@ -862,25 +840,6 @@ function ProviderFields({
|
|||
placeholder={`${provider.label} API key`}
|
||||
/>
|
||||
</Field>
|
||||
{provider.id === "openai_compatible" ? (
|
||||
<div className="mt-4">
|
||||
<Field label="Base URL" hint="Required for OpenAI-compatible providers.">
|
||||
<input
|
||||
value={current.openaiBaseUrl}
|
||||
onChange={(event) =>
|
||||
onChange({
|
||||
...value,
|
||||
[provider.id]: {
|
||||
...current,
|
||||
openaiBaseUrl: event.target.value,
|
||||
},
|
||||
})}
|
||||
className={INPUT_CLASS}
|
||||
placeholder="https://api.example.com/v1"
|
||||
/>
|
||||
</Field>
|
||||
</div>
|
||||
) : null}
|
||||
</div>
|
||||
);
|
||||
})}
|
||||
|
|
@ -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;
|
||||
|
|
|
|||
13
apps/fabro-web/app/install-config.test.ts
Normal file
13
apps/fabro-web/app/install-config.test.ts
Normal file
|
|
@ -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",
|
||||
]);
|
||||
});
|
||||
});
|
||||
17
apps/fabro-web/app/install-config.ts
Normal file
17
apps/fabro-web/app/install-config.ts
Normal file
|
|
@ -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;
|
||||
38
apps/fabro-web/app/install-flow.test.ts
Normal file
38
apps/fabro-web/app/install-flow.test.ts
Normal file
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
9
apps/fabro-web/app/install-flow.ts
Normal file
9
apps/fabro-web/app/install-flow.ts
Normal file
|
|
@ -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";
|
||||
}
|
||||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -128,10 +128,8 @@ struct LlmProvidersInput {
|
|||
|
||||
#[derive(Clone, Debug, Deserialize, Serialize)]
|
||||
struct LlmProviderInput {
|
||||
provider: Provider,
|
||||
api_key: String,
|
||||
#[serde(default)]
|
||||
openai_base_url: Option<String>,
|
||||
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<String>,
|
||||
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<InstallAppState>,
|
||||
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::<Vec<_>>()
|
||||
})
|
||||
},
|
||||
|
|
@ -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<String> {
|
||||
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<String> {
|
||||
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(),
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
);
|
||||
}
|
||||
|
|
@ -1,4 +1,5 @@
|
|||
mod install;
|
||||
mod install_openai_compatible;
|
||||
mod routing;
|
||||
mod runs;
|
||||
mod settings;
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue