fix(install): cover follow-up edge cases

Harden the remaining install flow regressions and add the missing
coverage for startup dispatch, finish-time shutdown behavior, and
partial-state persistence after vault failures.
This commit is contained in:
Bryan Helmkamp 2026-04-19 13:32:46 -04:00
parent 75f8ed845b
commit 3f21644d80
No known key found for this signature in database
5 changed files with 314 additions and 92 deletions

View file

@ -92,7 +92,7 @@ export default function InstallApp() {
return;
}
}
setSaveError(null);
setSaveError((current) => (current === null ? current : null));
}, [location.pathname]);
useEffect(() => {
@ -162,9 +162,13 @@ export default function InstallApp() {
setTimedOut(true);
}, 30_000);
const interval = window.setInterval(async () => {
const controller = new AbortController();
let inFlight = false;
const poll = async () => {
if (inFlight || controller.signal.aborted) return;
inFlight = true;
try {
const response = await fetch("/health");
const response = await fetch("/health", { signal: controller.signal });
const body = response.ok
? ((await response.json()) as { mode?: string })
: undefined;
@ -178,13 +182,18 @@ export default function InstallApp() {
window.location.href = finishState.restart_url;
}
} catch {
if (controller.signal.aborted) return;
if (shouldRedirectAfterHealthPoll({ kind: "error" })) {
window.location.href = finishState.restart_url;
}
} finally {
inFlight = false;
}
}, 1_000);
};
const interval = window.setInterval(poll, 2_000);
return () => {
controller.abort();
window.clearTimeout(deadline);
window.clearInterval(interval);
};
@ -277,9 +286,9 @@ export default function InstallApp() {
setSubmitting(true);
setSaveError(null);
try {
for (const provider of providers) {
await testInstallLlm(installToken, provider);
}
await Promise.all(
providers.map((provider) => testInstallLlm(installToken, provider)),
);
await putInstallLlm(installToken, providers);
const nextSession = await getInstallSession(installToken);
setSession(nextSession);
@ -829,7 +838,7 @@ function ProviderFields({
onChange: (nextValue: ProviderSelection) => void;
}) {
return (
<div className="space-y-4">
<div className="space-y-4">
{INSTALL_PROVIDERS.map((provider) => {
const current = value[provider.id] ?? { apiKey: "" };
return (

View file

@ -112,7 +112,7 @@ fn start_already_running_exits_with_error() {
clippy::disallowed_methods,
reason = "This integration test needs the real foreground process to verify install-mode startup behavior."
)]
fn start_without_settings_enters_install_mode_in_foreground() {
fn start_without_default_settings_enters_install_mode_in_foreground() {
let home_dir = tempfile::tempdir_in("/tmp").unwrap();
let storage_root = isolated_storage_dir();
let storage_dir = storage_root.path().join("storage");
@ -197,6 +197,101 @@ fn start_without_settings_ignores_no_web_during_install() {
);
}
#[test]
fn start_with_missing_explicit_flag_config_errors_without_entering_install_mode() {
let context = test_context!();
let missing_config = tempfile::tempdir_in("/tmp")
.unwrap()
.path()
.join("missing-settings.toml");
let mut filters = context.filters();
filters.push((
regex::escape(missing_config.to_str().unwrap()),
"[MISSING_CONFIG]".to_string(),
));
let mut explicit_cmd = context.command();
explicit_cmd.args([
"server",
"start",
"--config",
missing_config.to_str().unwrap(),
]);
fabro_snapshot!(filters.clone(), explicit_cmd, @"
success: false
exit_code: 1
----- stdout -----
----- stderr -----
error: reading config file [MISSING_CONFIG]: No such file or directory (os error 2)
> No such file or directory (os error 2)
");
}
#[test]
fn start_with_missing_env_config_errors_without_entering_install_mode() {
let context = test_context!();
let missing_config = tempfile::tempdir_in("/tmp")
.unwrap()
.path()
.join("missing-settings.toml");
let mut filters = context.filters();
filters.push((
regex::escape(missing_config.to_str().unwrap()),
"[MISSING_CONFIG]".to_string(),
));
let mut env_cmd = context.command();
env_cmd
.env("FABRO_CONFIG", &missing_config)
.args(["server", "start"]);
fabro_snapshot!(filters, env_cmd, @"
success: false
exit_code: 1
----- stdout -----
----- stderr -----
error: reading config file [MISSING_CONFIG]: No such file or directory (os error 2)
> No such file or directory (os error 2)
");
}
#[test]
fn start_with_malformed_default_settings_errors_without_entering_install_mode() {
let context = test_context!();
let settings_dir = context.home_dir.join(".fabro");
std::fs::create_dir_all(&settings_dir).unwrap();
let settings_path = settings_dir.join("settings.toml");
std::fs::write(&settings_path, "[server.listen\n").unwrap();
let mut filters = context.filters();
filters.push((
regex::escape(settings_path.to_str().unwrap()),
"[SETTINGS_PATH]".to_string(),
));
let mut cmd = context.command();
cmd.args(["server", "start"]);
fabro_snapshot!(filters, cmd, @"
success: false
exit_code: 1
----- stdout -----
----- stderr -----
error: Failed to parse settings file at [HOME_DIR]/.fabro/settings.toml: settings file is not valid TOML: TOML parse error at line 1, column 15
|
1 | [server.listen
| ^
invalid table header
expected `.`, `]`
> settings file is not valid TOML: TOML parse error at line 1, column 15
> |
> 1 | [server.listen
> | ^
> invalid table header
> expected `.`, `]`
");
}
#[test]
fn start_without_bind_uses_home_socket_instead_of_storage_socket() {
let context = test_context!();

View file

@ -40,6 +40,7 @@ pub struct InstallAppState {
pending_install: Arc<Mutex<PendingInstall>>,
storage_dir: Arc<PathBuf>,
config_path: Arc<PathBuf>,
home: Option<Home>,
install_listen: Arc<Mutex<InstallListenConfig>>,
first_operator: Arc<Mutex<Option<InstallOperatorFingerprint>>>,
finish_in_progress: Arc<AtomicBool>,
@ -73,6 +74,7 @@ impl InstallAppState {
pending_install: Arc::new(Mutex::new(PendingInstall::default())),
storage_dir: Arc::new(storage_dir.to_path_buf()),
config_path: Arc::new(config_path.to_path_buf()),
home: None,
install_listen: Arc::new(Mutex::new(InstallListenConfig::Tcp(
DEFAULT_INSTALL_TCP_LISTEN_ADDRESS.to_string(),
))),
@ -96,6 +98,7 @@ impl InstallAppState {
pending_install: Arc::new(Mutex::new(PendingInstall::default())),
storage_dir: Arc::new(storage_dir.to_path_buf()),
config_path: Arc::new(config_path.to_path_buf()),
home: None,
install_listen: Arc::new(Mutex::new(InstallListenConfig::Tcp(
DEFAULT_INSTALL_TCP_LISTEN_ADDRESS.to_string(),
))),
@ -106,13 +109,20 @@ impl InstallAppState {
}
}
fn with_finish_callback(self, on_finish: Arc<dyn Fn() + Send + Sync>) -> Self {
#[must_use]
pub fn with_finish_callback(self, on_finish: Arc<dyn Fn() + Send + Sync>) -> Self {
Self {
on_finish: Some(on_finish),
..self
}
}
#[must_use]
pub fn with_home(mut self, home: Home) -> Self {
self.home = Some(home);
self
}
#[must_use]
pub fn with_provider_base_url(
mut self,
@ -353,10 +363,10 @@ impl Drop for InstallFinishGuard {
pub async fn serve_install_command<F>(
bind_request: BindRequest,
state: InstallAppState,
mut on_ready: F,
on_ready: F,
) -> anyhow::Result<()>
where
F: FnMut(&Bind) -> anyhow::Result<()>,
F: FnOnce(&Bind) -> anyhow::Result<()>,
{
let (shutdown_tx, shutdown_rx) = watch::channel(false);
let finish_callback: Arc<dyn Fn() + Send + Sync> = Arc::new(move || {
@ -838,7 +848,7 @@ async fn post_install_finish(
return install_error_response(StatusCode::INTERNAL_SERVER_ERROR, err.to_string());
}
};
let home = Home::from_env();
let home = state.home.clone().unwrap_or_else(Home::from_env);
let dev_token = match dev_token::load_or_create_dev_token(&home.dev_token_path()) {
Ok(value) => value,
Err(err) => {
@ -1150,22 +1160,10 @@ async fn validate_llm_provider(
state: &InstallAppState,
input: &InstallLlmTestInput,
) -> Result<(), String> {
let base_url = provider_base_url(state, input.provider);
let client = install_http_client_for_url(&base_url)?;
let request = match input.provider {
Provider::Anthropic => client
.get(format!("{base_url}/models"))
.header("x-api-key", &input.api_key)
.header("anthropic-version", "2023-06-01")
.header("User-Agent", "fabro-server"),
Provider::OpenAi => client
.get(format!("{base_url}/models"))
.header("Authorization", format!("Bearer {}", input.api_key))
.header("User-Agent", "fabro-server"),
Provider::Gemini => client
.get(format!("{base_url}/models"))
.header("x-goog-api-key", &input.api_key)
.header("User-Agent", "fabro-server"),
let (auth_header, auth_value) = match input.provider {
Provider::Anthropic => ("x-api-key", input.api_key.clone()),
Provider::OpenAi => ("Authorization", format!("Bearer {}", input.api_key)),
Provider::Gemini => ("x-goog-api-key", input.api_key.clone()),
Provider::Kimi
| Provider::Zai
| Provider::Minimax
@ -1178,6 +1176,16 @@ async fn validate_llm_provider(
}
};
let base_url = provider_base_url(state, input.provider);
let client = install_http_client_for_url(&base_url)?;
let mut request = client
.get(format!("{base_url}/models"))
.header(auth_header, auth_value)
.header("User-Agent", "fabro-server");
if matches!(input.provider, Provider::Anthropic) {
request = request.header("anthropic-version", "2023-06-01");
}
let response = request
.timeout(Duration::from_secs(10))
.send()

View file

@ -1,4 +1,5 @@
use std::path::{Path, PathBuf};
use std::sync::OnceLock;
use axum::body::Body;
use axum::http::{HeaderMap, HeaderValue, StatusCode, header};
@ -15,15 +16,29 @@ pub fn serve_install(path: &str, headers: &HeaderMap) -> Response {
}
pub(crate) fn assert_install_mode_shell_ready() {
let index = load_asset("index.html").expect("install-mode SPA shell asset missing");
let injected = inject_install_mode(index);
let html = String::from_utf8(injected).expect("install-mode SPA shell must be valid UTF-8");
let shell = cached_install_mode_shell().clone().unwrap_or_else(|| {
load_injected_install_shell().expect("install-mode SPA shell asset missing")
});
let html = String::from_utf8(shell).expect("install-mode SPA shell must be valid UTF-8");
assert!(
html.contains(INSTALL_MODE_MARKER),
"install-mode SPA shell marker missing after injection"
);
}
fn cached_install_mode_shell() -> Option<Vec<u8>> {
if cfg!(debug_assertions) {
// In debug builds the SPA is reloaded from disk on every request.
return load_injected_install_shell();
}
static SHELL: OnceLock<Option<Vec<u8>>> = OnceLock::new();
SHELL.get_or_init(load_injected_install_shell).clone()
}
fn load_injected_install_shell() -> Option<Vec<u8>> {
Some(inject_install_mode(load_asset("index.html")?))
}
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
enum SpaMode {
Normal,
@ -88,16 +103,16 @@ fn load_asset(path: &str) -> Option<Vec<u8>> {
}
fn load_asset_for_mode(path: &str, mode: SpaMode) -> Option<Vec<u8>> {
let asset = load_asset(path)?;
if mode == SpaMode::Install && path == "index.html" {
return Some(inject_install_mode(asset));
return cached_install_mode_shell();
}
Some(asset)
load_asset(path)
}
fn inject_install_mode(bytes: Vec<u8>) -> Vec<u8> {
let Ok(html) = String::from_utf8(bytes.clone()) else {
return bytes;
let html = match String::from_utf8(bytes) {
Ok(html) => html,
Err(err) => return err.into_bytes(),
};
if html.contains(INSTALL_MODE_MARKER) {
return html.into_bytes();

View file

@ -1,14 +1,73 @@
use std::sync::Arc;
use std::sync::atomic::{AtomicBool, Ordering};
use std::time::Duration;
use axum::body::Body;
use axum::http::{Request, StatusCode};
use fabro_config::{Storage, parse_settings_layer, resolve_server_from_file};
use fabro_model::Provider;
use fabro_server::install::{InstallAppState, build_install_router};
use fabro_util::{Home, dev_token};
use fabro_vault::Vault;
use httpmock::MockServer;
use tokio::time::sleep;
use tower::ServiceExt;
use crate::helpers::body_json;
async fn configure_token_install(app: &axum::Router, token: &str) {
let llm_response = app
.clone()
.oneshot(
Request::builder()
.method("PUT")
.uri("/install/llm")
.header("authorization", format!("Bearer {token}"))
.header("content-type", "application/json")
.body(Body::from(
r#"{"providers":[{"provider":"anthropic","api_key":"anthropic-test-key"}]}"#,
))
.unwrap(),
)
.await
.unwrap();
assert_eq!(llm_response.status(), StatusCode::NO_CONTENT);
let server_response = app
.clone()
.oneshot(
Request::builder()
.method("PUT")
.uri("/install/server")
.header("authorization", format!("Bearer {token}"))
.header("content-type", "application/json")
.body(Body::from(
r#"{"canonical_url":"https://fabro.example.com"}"#,
))
.unwrap(),
)
.await
.unwrap();
assert_eq!(server_response.status(), StatusCode::NO_CONTENT);
let github_response = app
.clone()
.oneshot(
Request::builder()
.method("PUT")
.uri("/install/github/token")
.header("authorization", format!("Bearer {token}"))
.header("content-type", "application/json")
.body(Body::from(
r#"{"token":"ghp_test_token","username":"brynary"}"#,
))
.unwrap(),
)
.await
.unwrap();
assert_eq!(github_response.status(), StatusCode::NO_CONTENT);
}
#[tokio::test]
async fn install_router_isolated_from_normal_api_surface() {
let app = build_install_router(InstallAppState::for_test("test-install-token"));
@ -320,6 +379,41 @@ async fn token_install_finish_persists_settings_env_and_vault() {
assert_eq!(vault.get("GITHUB_TOKEN"), Some("ghp_test_token"));
}
#[tokio::test]
async fn token_install_finish_invokes_shutdown_callback_after_accepting() {
let temp_dir = tempfile::tempdir().unwrap();
let home_root = tempfile::tempdir().unwrap();
let config_path = temp_dir.path().join("settings.toml");
let callback_invoked = Arc::new(AtomicBool::new(false));
let callback_flag = Arc::clone(&callback_invoked);
let app = build_install_router(
InstallAppState::for_test_with_paths("test-install-token", temp_dir.path(), &config_path)
.with_home(Home::new(home_root.path().join(".fabro")))
.with_finish_callback(Arc::new(move || {
callback_flag.store(true, Ordering::Release);
})),
);
configure_token_install(&app, "test-install-token").await;
let finish_response = app
.oneshot(
Request::builder()
.method("POST")
.uri("/install/finish")
.header("authorization", "Bearer test-install-token")
.body(Body::empty())
.unwrap(),
)
.await
.unwrap();
assert_eq!(finish_response.status(), StatusCode::ACCEPTED);
assert!(!callback_invoked.load(Ordering::Acquire));
sleep(Duration::from_millis(650)).await;
assert!(callback_invoked.load(Ordering::Acquire));
}
#[tokio::test]
async fn install_validation_endpoints_validate_credentials_and_github_token() {
let llm_mock = MockServer::start_async().await;
@ -902,6 +996,7 @@ async fn install_server_rejects_trailing_slash_canonical_urls() {
#[tokio::test]
async fn install_finish_failure_restores_settings_and_vault_but_leaves_env_keys() {
let temp_dir = tempfile::tempdir().unwrap();
let home_root = tempfile::tempdir().unwrap();
let config_path = temp_dir.path().join("settings.toml");
std::fs::write(&config_path, "_version = 1\n[project]\nname = \"keep\"\n").unwrap();
@ -909,63 +1004,18 @@ async fn install_finish_failure_restores_settings_and_vault_but_leaves_env_keys(
let vault_path = storage.secrets_path();
std::fs::create_dir_all(vault_path.parent().unwrap()).unwrap();
std::fs::write(&vault_path, "{ not valid json").unwrap();
let callback_invoked = Arc::new(AtomicBool::new(false));
let callback_flag = Arc::clone(&callback_invoked);
let app = build_install_router(InstallAppState::for_test_with_paths(
"test-install-token",
temp_dir.path(),
&config_path,
));
let app = build_install_router(
InstallAppState::for_test_with_paths("test-install-token", temp_dir.path(), &config_path)
.with_home(Home::new(home_root.path().join(".fabro")))
.with_finish_callback(Arc::new(move || {
callback_flag.store(true, Ordering::Release);
})),
);
let llm_response = app
.clone()
.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":"anthropic","api_key":"anthropic-test-key"}]}"#,
))
.unwrap(),
)
.await
.unwrap();
assert_eq!(llm_response.status(), StatusCode::NO_CONTENT);
let server_response = app
.clone()
.oneshot(
Request::builder()
.method("PUT")
.uri("/install/server")
.header("authorization", "Bearer test-install-token")
.header("content-type", "application/json")
.body(Body::from(
r#"{"canonical_url":"https://fabro.example.com"}"#,
))
.unwrap(),
)
.await
.unwrap();
assert_eq!(server_response.status(), StatusCode::NO_CONTENT);
let github_response = app
.clone()
.oneshot(
Request::builder()
.method("PUT")
.uri("/install/github/token")
.header("authorization", "Bearer test-install-token")
.header("content-type", "application/json")
.body(Body::from(
r#"{"token":"ghp_test_token","username":"brynary"}"#,
))
.unwrap(),
)
.await
.unwrap();
assert_eq!(github_response.status(), StatusCode::NO_CONTENT);
configure_token_install(&app, "test-install-token").await;
let finish_response = app
.clone()
@ -1012,6 +1062,7 @@ async fn install_finish_failure_restores_settings_and_vault_but_leaves_env_keys(
let server_env = std::fs::read_to_string(storage.server_state().env_path()).unwrap();
assert!(server_env.contains("SESSION_SECRET="));
assert!(server_env.contains("FABRO_DEV_TOKEN="));
assert!(!callback_invoked.load(Ordering::Acquire));
let session_response = app
.oneshot(
@ -1034,3 +1085,47 @@ async fn install_finish_failure_restores_settings_and_vault_but_leaves_env_keys(
.any(|value| value == "github")
);
}
#[tokio::test]
async fn install_finish_failure_leaves_home_dev_token_mirror_written() {
let temp_dir = tempfile::tempdir().unwrap();
let home_root = tempfile::tempdir().unwrap();
let home = Home::new(home_root.path().join(".fabro"));
let config_path = temp_dir.path().join("settings.toml");
std::fs::write(&config_path, "_version = 1\n[project]\nname = \"keep\"\n").unwrap();
let storage = Storage::new(temp_dir.path());
let vault_path = storage.secrets_path();
std::fs::create_dir_all(vault_path.parent().unwrap()).unwrap();
std::fs::write(&vault_path, "{ not valid json").unwrap();
let app = build_install_router(
InstallAppState::for_test_with_paths("test-install-token", temp_dir.path(), &config_path)
.with_home(home.clone()),
);
configure_token_install(&app, "test-install-token").await;
let finish_response = app
.oneshot(
Request::builder()
.method("POST")
.uri("/install/finish")
.header("authorization", "Bearer test-install-token")
.body(Body::empty())
.unwrap(),
)
.await
.unwrap();
assert_eq!(finish_response.status(), StatusCode::INTERNAL_SERVER_ERROR);
let home_dev_token = dev_token::read_dev_token_file(&home.dev_token_path())
.expect("home dev token should exist");
let storage_dev_token =
dev_token::read_dev_token_file(&storage.server_state().dev_token_path())
.expect("storage dev token should exist");
assert_eq!(home_dev_token, storage_dev_token);
let server_env = std::fs::read_to_string(storage.server_state().env_path()).unwrap();
assert!(server_env.contains(&format!("FABRO_DEV_TOKEN={home_dev_token}")));
}