mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-08 03:10:26 +00:00
refactor(cli): collapse duplicate server-settings resolvers through local_server
Route install/uninstall through local_server::storage_dir instead of hand- rolled copies, drop dead connect_api_client and run_dir plumbing, eliminate double-resolve in prepare_server_bootstrap, and tighten the boundary allowlist now that uninstall no longer needs the exemption. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
5b1c40764d
commit
6e07f688ab
9 changed files with 11 additions and 82 deletions
|
|
@ -6,7 +6,6 @@ cd "$(dirname "$0")/../.."
|
|||
symbol_allowlist=(
|
||||
"lib/crates/fabro-cli/src/local_server.rs"
|
||||
"lib/crates/fabro-cli/src/commands/install.rs"
|
||||
"lib/crates/fabro-cli/src/commands/uninstall.rs"
|
||||
"lib/crates/fabro-cli/src/commands/run/runner.rs"
|
||||
"lib/crates/fabro-cli/src/commands/pr/mod.rs"
|
||||
"lib/crates/fabro-cli/src/commands/pr/create.rs"
|
||||
|
|
|
|||
|
|
@ -5,7 +5,7 @@
|
|||
|
||||
use std::future::Future;
|
||||
use std::net::SocketAddr;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::path::Path;
|
||||
use std::process::Stdio;
|
||||
use std::time::Duration;
|
||||
|
||||
|
|
@ -57,7 +57,7 @@ use crate::shared::provider_auth::{
|
|||
ApiKeySource, authenticate_provider, authenticate_provider_with_api_key_source,
|
||||
authenticate_provider_with_method, prompt_confirm, prompt_password, provider_display_name,
|
||||
};
|
||||
use crate::{server_client, user_config};
|
||||
use crate::{local_server, server_client, user_config};
|
||||
|
||||
const GITHUB_TOKEN_SECRET_KEY: &str = "GITHUB_TOKEN";
|
||||
const GITHUB_APP_PRIVATE_KEY_KEY: &str = "GITHUB_APP_PRIVATE_KEY";
|
||||
|
|
@ -1292,22 +1292,6 @@ fn render_server_resolve_errors(errors: Vec<ResolveError>) -> anyhow::Error {
|
|||
)
|
||||
}
|
||||
|
||||
fn resolved_server_storage_dir(settings: &SettingsLayer) -> Result<PathBuf> {
|
||||
let resolved =
|
||||
fabro_config::resolve_server_from_file(settings).map_err(render_server_resolve_errors)?;
|
||||
let resolved_root = resolved
|
||||
.storage
|
||||
.root
|
||||
.resolve(|name| std::env::var(name).ok())
|
||||
.map_err(|err| {
|
||||
anyhow::anyhow!(
|
||||
"failed to resolve {}: {err}",
|
||||
resolved.storage.root.as_source()
|
||||
)
|
||||
})?;
|
||||
Ok(PathBuf::from(resolved_root.value))
|
||||
}
|
||||
|
||||
async fn write_artifact_store_metadata(
|
||||
settings: &SettingsLayer,
|
||||
fabro_version: &str,
|
||||
|
|
@ -1510,9 +1494,7 @@ async fn run_install_github_inner(
|
|||
.context("failed to parse existing settings.toml")?,
|
||||
args.storage_dir.as_deref(),
|
||||
);
|
||||
// Install is a documented server-host exception: it may read [server.*]
|
||||
// directly.
|
||||
let storage_dir = resolved_server_storage_dir(&parsed_settings).unwrap_or_else(|_| {
|
||||
let storage_dir = local_server::storage_dir(&parsed_settings).unwrap_or_else(|_| {
|
||||
args.storage_dir
|
||||
.clone_path()
|
||||
.unwrap_or_else(default_storage_dir)
|
||||
|
|
@ -1659,9 +1641,7 @@ async fn run_install_inner(
|
|||
let s = Styles::detect_stderr();
|
||||
let emoji = console::Emoji("⚒️ ", "");
|
||||
let cli_settings = user_config::load_settings_with_storage_dir(args.storage_dir.as_deref())?;
|
||||
// Install is a documented server-host exception: it may read [server.*]
|
||||
// directly.
|
||||
let storage_dir = resolved_server_storage_dir(&cli_settings)?;
|
||||
let storage_dir = local_server::storage_dir(&cli_settings)?;
|
||||
let server_was_running = record::active_server_record(&storage_dir)?.is_some();
|
||||
let fabro_dir = fabro_util::Home::from_env().root().to_path_buf();
|
||||
let config_path = fabro_dir.join(SETTINGS_CONFIG_FILENAME);
|
||||
|
|
|
|||
|
|
@ -105,8 +105,6 @@ pub(super) async fn create_command(
|
|||
);
|
||||
}
|
||||
|
||||
// boundary-exempt(pr-api): remove with follow-up #1 when PR ops move
|
||||
// server-side.
|
||||
let vault = user_config::storage_dir(ctx.machine_settings())
|
||||
.ok()
|
||||
.and_then(|dir| Vault::load(Storage::new(&dir).secrets_path()).ok())
|
||||
|
|
|
|||
|
|
@ -47,8 +47,6 @@ fn load_github_credentials_required(
|
|||
printer: Printer,
|
||||
) -> Result<GitHubCredentials> {
|
||||
let ctx = CommandContext::base(printer, cli.clone(), cli_layer)?;
|
||||
// boundary-exempt(pr-api): remove with follow-up #1 when PR ops move
|
||||
// server-side.
|
||||
let server_settings =
|
||||
fabro_config::resolve_server_from_file(ctx.machine_settings()).map_err(|errors| {
|
||||
anyhow!(
|
||||
|
|
@ -60,8 +58,6 @@ fn load_github_credentials_required(
|
|||
.join("\n")
|
||||
)
|
||||
})?;
|
||||
// boundary-exempt(pr-api): remove with follow-up #1 when PR ops move
|
||||
// server-side.
|
||||
let vault = user_config::storage_dir(ctx.machine_settings())
|
||||
.ok()
|
||||
.and_then(|dir| fabro_vault::Vault::load(Storage::new(&dir).secrets_path()).ok());
|
||||
|
|
|
|||
|
|
@ -17,9 +17,7 @@ use fabro_workflow::records::Conclusion;
|
|||
use indicatif::HumanDuration;
|
||||
|
||||
use crate::server_client;
|
||||
use crate::shared::{
|
||||
format_tokens_human, format_usd_micros, print_diagnostics, relative_path, tilde_path,
|
||||
};
|
||||
use crate::shared::{format_tokens_human, format_usd_micros, print_diagnostics, relative_path};
|
||||
|
||||
pub(crate) fn print_preflight_workflow_summary(
|
||||
workflow: &types::PreflightWorkflowSummary,
|
||||
|
|
@ -151,7 +149,6 @@ pub(crate) async fn print_run_summary_with_client(
|
|||
&conclusion,
|
||||
run_id,
|
||||
None,
|
||||
None,
|
||||
pr_url.as_deref(),
|
||||
styles,
|
||||
printer,
|
||||
|
|
@ -166,7 +163,6 @@ pub(crate) async fn print_run_summary_with_client(
|
|||
pub(crate) fn print_run_conclusion(
|
||||
conclusion: &Conclusion,
|
||||
run_id: impl std::fmt::Display,
|
||||
run_dir: Option<&Path>,
|
||||
pushed_branch: Option<&str>,
|
||||
pr_url: Option<&str>,
|
||||
styles: &Styles,
|
||||
|
|
@ -248,16 +244,6 @@ pub(crate) fn print_run_conclusion(
|
|||
}
|
||||
}
|
||||
|
||||
if let Some(run_dir) = run_dir {
|
||||
fabro_util::printerr!(
|
||||
printer,
|
||||
"{}",
|
||||
styles
|
||||
.dim
|
||||
.apply_to(format!("Run: {}", tilde_path(run_dir)))
|
||||
);
|
||||
}
|
||||
|
||||
if let Some(ref failure) = conclusion.failure_reason {
|
||||
fabro_util::printerr!(printer, "Failure: {}", styles.red.apply_to(failure));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -23,7 +23,7 @@ use tracing::warn;
|
|||
use crate::args::UninstallArgs;
|
||||
use crate::commands::server;
|
||||
use crate::shared::{format_size, print_json_pretty, tilde_path};
|
||||
use crate::user_config;
|
||||
use crate::{local_server, user_config};
|
||||
|
||||
#[derive(Debug, Serialize)]
|
||||
struct Inventory {
|
||||
|
|
@ -59,25 +59,10 @@ pub(crate) async fn run_uninstall(
|
|||
return Ok(());
|
||||
}
|
||||
|
||||
let storage_dir = user_config::load_settings().map_or_else(
|
||||
|_| user_config::default_storage_dir(),
|
||||
|settings| {
|
||||
// Uninstall is a documented server-host exception: it may read
|
||||
// [server.*] directly.
|
||||
fabro_config::resolve_server_from_file(&settings)
|
||||
.ok()
|
||||
.and_then(|resolved| {
|
||||
resolved
|
||||
.storage
|
||||
.root
|
||||
.resolve(|name| std::env::var(name).ok())
|
||||
.ok()
|
||||
})
|
||||
.map_or_else(user_config::default_storage_dir, |resolved_root| {
|
||||
PathBuf::from(resolved_root.value)
|
||||
})
|
||||
},
|
||||
);
|
||||
let storage_dir = user_config::load_settings()
|
||||
.ok()
|
||||
.and_then(|settings| local_server::storage_dir(&settings).ok())
|
||||
.unwrap_or_else(user_config::default_storage_dir);
|
||||
|
||||
let inventory = build_inventory(&home_root, &storage_dir)?;
|
||||
|
||||
|
|
|
|||
|
|
@ -7,7 +7,6 @@
|
|||
use std::path::PathBuf;
|
||||
|
||||
use anyhow::Result;
|
||||
use fabro_config::ServerRuntimeState;
|
||||
use fabro_server::bind::BindRequest;
|
||||
use fabro_server::serve::resolve_bind_request_from_settings;
|
||||
use fabro_types::settings::{ServerAuthMethod, SettingsLayer};
|
||||
|
|
@ -39,10 +38,6 @@ pub(crate) fn storage_dir(settings: &SettingsLayer) -> Result<PathBuf> {
|
|||
Ok(PathBuf::from(resolved_root.value))
|
||||
}
|
||||
|
||||
pub(crate) fn runtime_state(settings: &SettingsLayer) -> Result<ServerRuntimeState> {
|
||||
Ok(ServerRuntimeState::new(storage_dir(settings)?))
|
||||
}
|
||||
|
||||
pub(crate) fn bind_request(
|
||||
settings: &SettingsLayer,
|
||||
cli_override: Option<&str>,
|
||||
|
|
|
|||
|
|
@ -481,7 +481,7 @@ async fn prepare_server_bootstrap(
|
|||
let settings =
|
||||
user_config::load_settings_with_config_and_storage_dir(config_path, storage_dir)?;
|
||||
let storage_dir = local_server::storage_dir(&settings)?;
|
||||
let runtime_state = local_server::runtime_state(&settings)?;
|
||||
let runtime_state = fabro_config::ServerRuntimeState::new(storage_dir.clone());
|
||||
let foreground_server_log_bootstrap = if foreground {
|
||||
Some(commands::server::start::prepare_foreground_server_log(&storage_dir).await?)
|
||||
} else {
|
||||
|
|
|
|||
|
|
@ -140,16 +140,6 @@ async fn connect_local_api_client_bundle(
|
|||
}
|
||||
}
|
||||
|
||||
#[allow(
|
||||
dead_code,
|
||||
reason = "Retained for pending storage-backed internal callers and referenced in existing design docs."
|
||||
)]
|
||||
pub(crate) async fn connect_api_client(storage_dir: &Path) -> Result<fabro_api::ApiClient> {
|
||||
connect_local_api_client_bundle(storage_dir, &user_config::active_settings_path(None))
|
||||
.await
|
||||
.map(|client| client.api_client())
|
||||
}
|
||||
|
||||
async fn connect_target_api_client_bundle(target: &ServerTarget) -> Result<Client> {
|
||||
let credential = resolve_target_credential(target, None, local_dev_token_fallback(target))?;
|
||||
let oauth_session = refreshable_oauth(target, credential.as_ref());
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue