fix(lint): clippy cleanup for Stage 3/4 consumer migration

- effective_settings server_defaults_layer: drop Result wrapper since
  the body never fails after the v2 switch
- merge.rs: allow needless_pass_by_value module-wide since every
  merge helper consumes both sides by design
- fabro-cli overrides.rs: replace &Option<String> sigs with
  Option<&str>, collapse Default-plus-assignment into struct literal
  (avoid clippy::field_reassign_with_default), and box the metadata
  HashMap inline
- fabro-cli manifest_builder.rs: pull DaytonaDockerfileLayer into
  scope so the pattern match stays absolute-path-clean
- fabro-cli main.rs + commands/config/mod.rs: Box::pin the settings
  subcommand future so clippy::large_futures stays happy
This commit is contained in:
Bryan Helmkamp 2026-04-09 10:16:47 -04:00
parent f4a79b8965
commit a6047250cf
No known key found for this signature in database
7 changed files with 40 additions and 35 deletions

View file

@ -80,7 +80,7 @@ async fn merged_config(args: &SettingsArgs) -> anyhow::Result<Settings> {
}
pub(crate) async fn execute(args: &SettingsArgs, globals: &GlobalArgs) -> anyhow::Result<()> {
let config = merged_config(args).await?;
let config = Box::pin(merged_config(args)).await?;
if globals.json {
print_json_pretty(&config)?;
return Ok(());

View file

@ -23,13 +23,13 @@ pub(crate) fn parse_labels(labels: &[String]) -> HashMap<String, String> {
.collect()
}
fn model_from_args(model: &Option<String>, provider: &Option<String>) -> Option<RunModelLayer> {
fn model_from_args(model: Option<&str>, provider: Option<&str>) -> Option<RunModelLayer> {
if model.is_none() && provider.is_none() {
return None;
}
Some(RunModelLayer {
provider: provider.as_deref().map(InterpString::parse),
name: model.as_deref().map(InterpString::parse),
provider: provider.map(InterpString::parse),
name: model.map(InterpString::parse),
fallbacks: Vec::new(),
})
}
@ -73,7 +73,7 @@ impl TryFrom<&RunArgs> for ConfigLayer {
type Error = anyhow::Error;
fn try_from(args: &RunArgs) -> Result<Self, Self::Error> {
let model = model_from_args(&args.model, &args.provider);
let model = model_from_args(args.model.as_deref(), args.provider.as_deref());
let sandbox = sandbox_layer(
args.sandbox.map(Into::into),
sparse_flag(args.preserve_sandbox),
@ -84,29 +84,29 @@ impl TryFrom<&RunArgs> for ConfigLayer {
sparse_flag(args.no_retro),
);
let mut metadata = parse_labels(&args.label);
// verbose is a CLI output concern in v2; staged via metadata for Stage 4.
if args.verbose {
metadata.insert("fabro.verbose".into(), "true".into());
}
let run = RunLayer {
goal: args.goal.as_deref().map(InterpString::parse),
metadata: parse_labels(&args.label),
metadata,
model,
sandbox,
execution,
..RunLayer::default()
};
let mut file = SettingsFile::default();
file.run = Some(run);
// goal_file is not part of v2; fall through to Settings.goal_file via the bridge.
// Stage 4 consumers that still consult goal_file read it from Settings.
let _ = &args.goal_file;
// verbose is a CLI output concern in v2; staged via metadata for Stage 4.
if args.verbose {
file.run
.as_mut()
.unwrap()
.metadata
.insert("fabro.verbose".into(), "true".into());
}
Ok(Self::from(file))
Ok(Self::from(SettingsFile {
run: Some(run),
..SettingsFile::default()
}))
}
}
@ -114,29 +114,30 @@ impl TryFrom<&PreflightArgs> for ConfigLayer {
type Error = anyhow::Error;
fn try_from(args: &PreflightArgs) -> Result<Self, Self::Error> {
let model = model_from_args(&args.model, &args.provider);
let model = model_from_args(args.model.as_deref(), args.provider.as_deref());
let sandbox = args.sandbox.map(|s| RunSandboxLayer {
provider: Some(SandboxProvider::from(s).to_string()),
..RunSandboxLayer::default()
});
let mut metadata = std::collections::HashMap::new();
if args.verbose {
metadata.insert("fabro.verbose".into(), "true".into());
}
let run = RunLayer {
goal: args.goal.as_deref().map(InterpString::parse),
metadata,
model,
sandbox,
..RunLayer::default()
};
let mut file = SettingsFile::default();
file.run = Some(run);
let _ = &args.goal_file; // Stage 4 preflight still reads goal_file via Settings bridge.
if args.verbose {
file.run
.as_mut()
.unwrap()
.metadata
.insert("fabro.verbose".into(), "true".into());
}
Ok(Self::from(file))
Ok(Self::from(SettingsFile {
run: Some(run),
..SettingsFile::default()
}))
}
}

View file

@ -230,7 +230,9 @@ async fn main_inner() -> (String, Result<()>) {
}
Commands::Pr(ns) => Box::pin(commands::pr::dispatch(ns, &globals)).await?,
Commands::Secret(ns) => commands::secret::dispatch(ns, &globals).await?,
Commands::Settings(args) => commands::config::execute(&args, &globals).await?,
Commands::Settings(args) => {
Box::pin(commands::config::execute(&args, &globals)).await?;
}
Commands::Workflow(ns) => commands::workflow::dispatch(ns, &globals)?,
Commands::Upgrade(args) => {
commands::upgrade::run_upgrade(args, &globals).await?;

View file

@ -10,6 +10,7 @@ use fabro_config::user::active_settings_path;
use fabro_graphviz::graph::AttrValue;
use fabro_graphviz::parser;
use fabro_sandbox::daytona::detect_repo_info;
use fabro_types::settings::v2::run::DaytonaDockerfileLayer;
use fabro_types::{RunId, Settings};
use fabro_workflow::git::{GitSyncStatus, head_sha, sync_status};
@ -327,8 +328,7 @@ fn collect_workflow_config_files(
.and_then(|daytona| daytona.snapshot.as_ref())
.and_then(|snapshot| snapshot.dockerfile.as_ref());
let Some(fabro_types::settings::v2::run::DaytonaDockerfileLayer::Path { path }) = dockerfile
else {
let Some(DaytonaDockerfileLayer::Path { path }) = dockerfile else {
return Ok(());
};

View file

@ -71,7 +71,7 @@ pub fn resolve_settings(
let mut stripped_user = user;
strip_owner_domains(stripped_user.as_v2_mut());
let server_defaults = server_defaults_layer(server_settings)?;
let server_defaults = server_defaults_layer(server_settings);
let mut settings = args
.combine(workflow)
@ -101,13 +101,13 @@ fn strip_owner_domains(file: &mut SettingsFile) {
file.server = None;
}
fn server_defaults_layer(settings: &Settings) -> Result<Settings> {
fn server_defaults_layer(settings: &Settings) -> Settings {
let mut out = settings.clone();
// Run manifests carry their own dry-run intent. Do not let a daemon's
// startup-time fallback mode silently force every submitted run/preflight
// into simulation.
out.dry_run = None;
Ok(out)
out
}
fn apply_server_defaults(settings: &mut Settings, server: &Settings) {

View file

@ -5,6 +5,7 @@
//! sticky merge-by-key where the requirements call for it, splice-capable
//! string arrays, whole-list replacement for ordered prepare steps, and
//! ordered hook merging with optional `id` replacement.
#![allow(clippy::needless_pass_by_value)]
use std::collections::HashMap;

View file

@ -15,6 +15,7 @@ use fabro_llm::Provider;
use fabro_model::Catalog;
use fabro_sandbox::daytona::DaytonaConfig;
use fabro_sandbox::{DockerSandboxOptions, Sandbox, SandboxProvider, SandboxSpec};
use fabro_types::settings::v2::SettingsFile;
use fabro_types::settings::v2::interp::InterpString;
use fabro_types::settings::v2::run::{
AgentPermissions, ApprovalMode, DaytonaDockerfileLayer, RunExecutionLayer, RunLayer, RunMode,
@ -251,7 +252,7 @@ fn manifest_args_layer(args: Option<&types::ManifestArgs>) -> ConfigLayer {
..RunLayer::default()
});
let mut file = fabro_types::settings::v2::SettingsFile::default();
let mut file = SettingsFile::default();
if let Some(run) = run {
file.run = Some(run);
}