refactor(cli): collapse duplicated bootstrap and settings helpers

- Merge prepare_foreground_server_bootstrap and prepare_server_sink_bootstrap
  into one prepare_server_bootstrap(config, storage, foreground).
- Drop three one-line settings_layer_* passthroughs from user_config; callers
  now use load_settings_with_{storage_dir,config_and_storage_dir} directly.
- Swap underscore-prefixed lock field for #[expect(dead_code, reason=…)] to
  document RAII intent explicitly.
- Remove two narrate-what-it-does comments.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-04-19 16:40:33 -04:00
parent 858e8e1270
commit a256c14a77
No known key found for this signature in database
7 changed files with 34 additions and 58 deletions

View file

@ -21,10 +21,8 @@ fn config_layers(
Some(path) => workflow_and_project_layers(path, cwd)?,
None => (SettingsLayer::default(), load_settings_project(cwd)?),
};
let user_layer = user_config::settings_layer_with_config_and_storage_dir(
Some(ctx.base_config_path()),
None,
)?;
let user_layer =
user_config::load_settings_with_config_and_storage_dir(Some(ctx.base_config_path()), None)?;
Ok(EffectiveSettingsLayers::new(
SettingsLayer::default(),
workflow_layer,

View file

@ -7,7 +7,7 @@ use fabro_util::terminal::Styles;
use crate::args::RunArgs;
use crate::command_context::CommandContext;
use crate::shared::print_json_pretty;
use crate::user_config::settings_layer_with_storage_dir;
use crate::user_config::load_settings_with_storage_dir;
pub(crate) async fn execute(
mut args: RunArgs,
@ -17,7 +17,7 @@ pub(crate) async fn execute(
) -> Result<()> {
let styles: &'static Styles = Box::leak(Box::new(Styles::detect_stderr()));
let ctx = CommandContext::for_target(&args.target, printer, cli.clone(), cli_layer)?;
let cli_defaults = settings_layer_with_storage_dir(None)?;
let cli_defaults = load_settings_with_storage_dir(None)?;
args.verbose = args.verbose || cli.output.verbosity == OutputVerbosity::Verbose;
let quiet = args.detach;

View file

@ -8,7 +8,7 @@ use crate::args::{AttachArgs, RunCommands, RunWorkerArgs, StartArgs};
use crate::command_context::CommandContext;
use crate::server_runs::ServerSummaryLookup;
use crate::shared::print_json_pretty;
use crate::user_config::settings_layer_with_storage_dir;
use crate::user_config::load_settings_with_storage_dir;
pub(crate) mod attach;
pub(crate) mod command;
@ -39,7 +39,7 @@ pub(crate) async fn dispatch(
RunCommands::Run(args) => Box::pin(command::execute(args, cli, cli_layer, printer)).await,
RunCommands::Create(args) => {
let styles: &'static Styles = Box::leak(Box::new(Styles::detect_stderr()));
let cli_defaults = settings_layer_with_storage_dir(None)?;
let cli_defaults = load_settings_with_storage_dir(None)?;
let ctx = CommandContext::for_target(&args.target, printer, cli.clone(), cli_layer)?;
let created_run = Box::pin(create::create_run(
&ctx,

View file

@ -158,6 +158,6 @@ mod tests {
write_server_record(&path, &record).unwrap();
assert!(active_server_record(dir.path()).unwrap().is_none());
assert!(!path.exists()); // lazy cleanup removed file
assert!(!path.exists());
}
}

View file

@ -18,7 +18,8 @@ use tokio::time;
use super::record;
pub(crate) struct ForegroundServerLogBootstrap {
_lock_file: std::fs::File,
#[expect(dead_code, reason = "held for its Drop to release the server lock")]
lock_file: std::fs::File,
}
pub(crate) async fn execute(
@ -76,9 +77,7 @@ pub(crate) async fn prepare_foreground_server_log(
std::fs::File::create(&log_path)
.with_context(|| format!("creating server log file {}", log_path.display()))?;
Ok(ForegroundServerLogBootstrap {
_lock_file: lock_file,
})
Ok(ForegroundServerLogBootstrap { lock_file })
}
pub(crate) async fn ensure_server_running_for_storage(
@ -364,7 +363,7 @@ async fn execute_daemon(
printer: Printer,
) -> Result<()> {
let lock_file = acquire_lock(storage_dir).await?;
let _lock_file = lock_file; // keep alive until function returns
let _lock_file = lock_file;
if let Some(existing) = record::active_server_record(storage_dir)? {
if announce {

View file

@ -420,66 +420,58 @@ async fn pre_tracing_bootstrap(command: &Commands) -> Result<PreTracingBootstrap
Commands::Server(ServerNamespace {
command: ServerCommand::Start(args),
}) if args.foreground => {
prepare_foreground_server_bootstrap(
prepare_server_bootstrap(
args.serve_args.config.as_deref(),
args.storage_dir.as_deref(),
true,
)
.await
}
Commands::Server(ServerNamespace {
command: ServerCommand::Restart(args),
}) if args.foreground => {
prepare_foreground_server_bootstrap(
prepare_server_bootstrap(
args.serve_args.config.as_deref(),
args.storage_dir.as_deref(),
true,
)
.await
}
Commands::Server(ServerNamespace {
command: ServerCommand::Serve(args),
}) => prepare_server_sink_bootstrap(
args.serve_args.config.as_deref(),
args.storage_dir.as_deref(),
),
}) => {
prepare_server_bootstrap(
args.serve_args.config.as_deref(),
args.storage_dir.as_deref(),
false,
)
.await
}
_ => Ok(PreTracingBootstrap::cli()),
}
}
async fn prepare_foreground_server_bootstrap(
async fn prepare_server_bootstrap(
config_path: Option<&std::path::Path>,
storage_dir: Option<&std::path::Path>,
foreground: bool,
) -> Result<PreTracingBootstrap> {
let settings =
user_config::load_settings_with_config_and_storage_dir(config_path, storage_dir)?;
let storage_dir = user_config::storage_dir(&settings)?;
let storage = fabro_config::Storage::new(&storage_dir);
let foreground_server_log_bootstrap =
commands::server::start::prepare_foreground_server_log(&storage_dir).await?;
let foreground_server_log_bootstrap = if foreground {
Some(commands::server::start::prepare_foreground_server_log(&storage_dir).await?)
} else {
None
};
Ok(PreTracingBootstrap {
sink: logging::InternalLogSink::Server {
path: storage.server_state().log_path(),
},
config_log_level: server_config_log_level(&settings),
foreground_server_log_bootstrap: Some(foreground_server_log_bootstrap),
})
}
fn prepare_server_sink_bootstrap(
config_path: Option<&std::path::Path>,
storage_dir: Option<&std::path::Path>,
) -> Result<PreTracingBootstrap> {
let settings =
user_config::load_settings_with_config_and_storage_dir(config_path, storage_dir)?;
let storage_dir = user_config::storage_dir(&settings)?;
let storage = fabro_config::Storage::new(&storage_dir);
Ok(PreTracingBootstrap {
sink: logging::InternalLogSink::Server {
path: storage.server_state().log_path(),
},
config_log_level: server_config_log_level(&settings),
foreground_server_log_bootstrap: None,
foreground_server_log_bootstrap,
})
}

View file

@ -22,31 +22,18 @@ pub(crate) fn load_settings() -> anyhow::Result<SettingsLayer> {
load_settings_with_config_and_storage_dir(None, None)
}
pub(crate) fn settings_layer_with_config_and_storage_dir(
config_path: Option<&Path>,
storage_dir: Option<&Path>,
) -> anyhow::Result<SettingsLayer> {
let layer = load_settings_config(config_path)?;
Ok(apply_storage_dir_override(layer, storage_dir))
}
pub(crate) fn settings_layer_with_storage_dir(
storage_dir: Option<&Path>,
) -> anyhow::Result<SettingsLayer> {
settings_layer_with_config_and_storage_dir(None, storage_dir)
}
pub(crate) fn load_settings_with_storage_dir(
storage_dir: Option<&Path>,
) -> anyhow::Result<SettingsLayer> {
settings_layer_with_storage_dir(storage_dir)
load_settings_with_config_and_storage_dir(None, storage_dir)
}
pub(crate) fn load_settings_with_config_and_storage_dir(
config_path: Option<&Path>,
storage_dir: Option<&Path>,
) -> anyhow::Result<SettingsLayer> {
settings_layer_with_config_and_storage_dir(config_path, storage_dir)
let layer = load_settings_config(config_path)?;
Ok(apply_storage_dir_override(layer, storage_dir))
}
fn render_resolve_errors(errors: Vec<fabro_config::ResolveError>) -> anyhow::Error {