scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-08-28 05:27:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

refactor(cli): finish command context cleanup

Browse source
This commit is contained in:
Bryan Helmkamp 2026-04-23 07:14:32 -04:00
parent fbe0bdfbc1
commit 2b933597b2
No known key found for this signature in database
8 changed files with 312 additions and 76 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

193
lib/crates/fabro-cli/src/command_context.rs
View file

@ -43,7 +43,37 @@ impl CommandContext {
cli_layer: &CliLayer,
process_local_json: bool,
) -> Result<Self> {
Self::new(printer, process_local_json, ServerMode::None, cli_layer)
let (machine_settings, user_settings) = load_merged_settings(cli_layer, &ServerMode::None)?;
Self::base_with_settings(
printer,
cli_layer,
process_local_json,
machine_settings,
user_settings,
)
}
pub(crate) fn base_with_settings(
printer: Printer,
cli_layer: &CliLayer,
process_local_json: bool,
machine_settings: SettingsLayer,
user_settings: UserSettings,
) -> Result<Self> {
let cwd = std::env::current_dir().context("Failed to get current directory")?;
let base_config_path = user_config::active_settings_path(None);
Ok(Self {
printer,
process_local_json,
cwd,
base_config_path,
cli_layer: cli_layer.clone(),
machine_settings,
user_settings,
server_mode: ServerMode::None,
server: OnceCell::new(),
})
}
pub(crate) fn with_target(&self, args: &ServerTargetArgs) -> Result<Self> {
@ -59,29 +89,6 @@ impl CommandContext {
})
}
fn new(
printer: Printer,
process_local_json: bool,
server_mode: ServerMode,
cli_layer: &CliLayer,
) -> Result<Self> {
let cwd = std::env::current_dir().context("Failed to get current directory")?;
let base_config_path = user_config::active_settings_path(None);
let (machine_settings, user_settings) = load_merged_settings(cli_layer, &server_mode)?;
Ok(Self {
printer,
process_local_json,
cwd,
base_config_path,
cli_layer: cli_layer.clone(),
machine_settings,
user_settings,
server_mode,
server: OnceCell::new(),
})
}
pub(crate) fn printer(&self) -> Printer {
self.printer
}
@ -137,14 +144,28 @@ impl CommandContext {
}
fn with_server_mode(&self, server_mode: ServerMode) -> Result<Self> {
let (machine_settings, user_settings) = match &server_mode {
ServerMode::ByStorageDir { .. } => load_merged_settings(&self.cli_layer, &server_mode)?,
ServerMode::None | ServerMode::ByTarget { .. } => {
(self.machine_settings.clone(), self.user_settings.clone())
}
};
// Always reload settings for the requested derivation mode so the result
// depends only on the requested mode, not on whichever derived context
// happened to call into this helper.
let (machine_settings, user_settings) =
load_merged_settings(&self.cli_layer, &server_mode)?;
Ok(Self {
Ok(
self.with_server_mode_from_loaded_settings(
server_mode,
machine_settings,
user_settings,
),
)
}
fn with_server_mode_from_loaded_settings(
&self,
server_mode: ServerMode,
machine_settings: SettingsLayer,
user_settings: UserSettings,
) -> Self {
Self {
printer: self.printer,
process_local_json: self.process_local_json,
cwd: self.cwd.clone(),
@ -154,7 +175,7 @@ impl CommandContext {
user_settings,
server_mode,
server: OnceCell::new(),
})
}
}
}
@ -188,13 +209,14 @@ fn merge_settings_layer(
mod tests {
use std::path::PathBuf;
use fabro_config::parse_settings_layer;
use fabro_config::user::apply_storage_dir_override;
use fabro_config::{UserSettings, parse_settings_layer};
use fabro_types::settings::SettingsLayer;
use fabro_types::settings::cli::{CliLayer, CliOutputLayer, OutputFormat, OutputVerbosity};
use fabro_util::printer::Printer;
use tokio::sync::OnceCell;
use super::{CommandContext, ServerMode, merge_settings_layer};
use crate::args::ServerTargetArgs;
fn cli_layer_with_json_and_verbose() -> CliLayer {
CliLayer {
@ -225,6 +247,27 @@ mod tests {
}
}
fn synthetic_context_with_settings(
process_local_json: bool,
printer: Printer,
cli_layer: CliLayer,
machine_settings: SettingsLayer,
user_settings: UserSettings,
server_mode: ServerMode,
) -> CommandContext {
CommandContext {
printer,
process_local_json,
cwd: PathBuf::from("/tmp/workspace"),
base_config_path: PathBuf::from("/tmp/settings.toml"),
cli_layer,
machine_settings,
user_settings,
server_mode,
server: OnceCell::new(),
}
}
#[test]
fn context_exposes_resolved_output_and_explicit_json_state() {
let ctx = synthetic_context(true, Printer::Default);
@ -241,11 +284,13 @@ mod tests {
#[test]
fn deriving_target_context_preserves_invocation_state() {
let base = synthetic_context(true, Printer::Verbose);
let derived = base
.with_target(&ServerTargetArgs {
server: Some("https://fabro.example.com".to_string()),
})
.expect("target context should derive");
let derived = base.with_server_mode_from_loaded_settings(
ServerMode::ByTarget {
target_override: Some("https://fabro.example.com".to_string()),
},
base.machine_settings().clone(),
base.user_settings().clone(),
);
assert_eq!(derived.printer(), Printer::Verbose);
assert!(derived.explicit_json_requested());
@ -271,7 +316,7 @@ root = "/srv/fabro/default"
"#,
)
.expect("settings fixture should parse");
let override_disk_settings = fabro_config::user::apply_storage_dir_override(
let override_disk_settings = apply_storage_dir_override(
base_disk_settings.clone(),
Some(std::path::Path::new("/srv/fabro/override")),
);
@ -305,6 +350,61 @@ root = "/srv/fabro/default"
);
}
#[test]
fn deriving_target_from_connection_context_discards_storage_override() {
let cli_layer = cli_layer_with_json_and_verbose();
let base_disk_settings = parse_settings_layer(
r#"
_version = 1
[server.storage]
root = "/srv/fabro/default"
"#,
)
.expect("settings fixture should parse");
let override_disk_settings = apply_storage_dir_override(
base_disk_settings.clone(),
Some(std::path::Path::new("/srv/fabro/override")),
);
let (base_settings, base_user_settings) =
merge_settings_layer(base_disk_settings, &cli_layer)
.expect("base settings should merge");
let (connection_settings, connection_user_settings) =
merge_settings_layer(override_disk_settings, &cli_layer)
.expect("connection settings should merge");
let connection_ctx = synthetic_context_with_settings(
false,
Printer::Default,
cli_layer,
connection_settings,
connection_user_settings,
ServerMode::ByStorageDir {
target_override: None,
storage_dir_override: Some(PathBuf::from("/srv/fabro/override")),
},
);
let derived = connection_ctx.with_server_mode_from_loaded_settings(
ServerMode::ByTarget {
target_override: Some("https://fabro.example.com".to_string()),
},
base_settings,
base_user_settings,
);
assert_eq!(
derived
.machine_settings()
.server
.as_ref()
.and_then(|server| server.storage.as_ref())
.and_then(|storage| storage.root.as_ref())
.map(|root| root.as_source()),
Some("/srv/fabro/default".to_string())
);
}
#[test]
fn explicit_json_guard_uses_invocation_flag_not_resolved_output_format() {
let json_ctx = synthetic_context(true, Printer::Default);
@ -316,11 +416,14 @@ root = "/srv/fabro/default"
#[test]
fn target_resolution_errors_remain_deferred() {
let ctx = synthetic_context(false, Printer::Default)
.with_target(&ServerTargetArgs {
server: Some("not-a-valid-target".to_string()),
})
.expect("target derivation should not resolve the target eagerly");
let base = synthetic_context(false, Printer::Default);
let ctx = base.with_server_mode_from_loaded_settings(
ServerMode::ByTarget {
target_override: Some("not-a-valid-target".to_string()),
},
base.machine_settings().clone(),
base.user_settings().clone(),
);
assert!(matches!(ctx.server_mode, ServerMode::ByTarget { .. }));
}

12
lib/crates/fabro-cli/src/commands/run/command.rs
View file

@ -5,27 +5,17 @@ use fabro_util::terminal::Styles;
use crate::args::RunArgs;
use crate::command_context::CommandContext;
use crate::shared::print_json_pretty;
use crate::user_config::load_settings_with_storage_dir;
pub(crate) async fn execute(mut args: RunArgs, base_ctx: &CommandContext) -> Result<()> {
let styles: &'static Styles = Box::leak(Box::new(Styles::detect_stderr()));
let printer = base_ctx.printer();
let ctx = base_ctx.with_target(&args.target)?;
let cli_defaults = load_settings_with_storage_dir(None)?;
args.verbose =
args.verbose || ctx.user_settings().cli.output.verbosity == OutputVerbosity::Verbose;
let quiet = args.detach;
let prevent_idle_sleep = ctx.user_settings().cli.exec.prevent_idle_sleep;
let created_run = Box::pin(super::create::create_run(
&ctx,
&args,
cli_defaults,
styles,
quiet,
printer,
))
.await?;
let created_run = Box::pin(super::create::create_run(&ctx, &args, styles, quiet)).await?;
if !quiet {
fabro_util::printerr!(

5
lib/crates/fabro-cli/src/commands/run/create.rs
View file

@ -1,8 +1,6 @@
use fabro_config::load::load_settings_user;
use fabro_config::user::active_settings_path;
use fabro_types::RunId;
use fabro_types::settings::SettingsLayer;
use fabro_util::printer::Printer;
use fabro_util::terminal::Styles;
use super::output::{api_diagnostics_to_local, print_preflight_workflow_summary};
@ -22,10 +20,8 @@ pub(crate) struct CreatedRun {
pub(crate) async fn create_run(
ctx: &CommandContext,
args: &RunArgs,
_cli_defaults: SettingsLayer,
styles: &Styles,
quiet: bool,
printer: Printer,
) -> anyhow::Result<CreatedRun> {
let workflow_path = args
.workflow
@ -51,6 +47,7 @@ pub(crate) async fn create_run(
})?;
let client = ctx.server().await?;
if !quiet {
let printer = ctx.printer();
let preflight = client.run_preflight(built.manifest.clone()).await?;
let diagnostics = api_diagnostics_to_local(&preflight.workflow.diagnostics);
if !diagnostics

12
lib/crates/fabro-cli/src/commands/run/mod.rs
View file

@ -5,7 +5,6 @@ use fabro_util::terminal::Styles;
use crate::args::{AttachArgs, RunCommands, RunWorkerArgs, StartArgs};
use crate::command_context::CommandContext;
use crate::shared::print_json_pretty;
use crate::user_config::load_settings_with_storage_dir;
pub(crate) mod attach;
pub(crate) mod command;
@ -32,17 +31,8 @@ pub(crate) async fn dispatch(cmd: RunCommands, base_ctx: &CommandContext) -> Res
RunCommands::Run(args) => Box::pin(command::execute(args, base_ctx)).await,
RunCommands::Create(args) => {
let styles: &'static Styles = Box::leak(Box::new(Styles::detect_stderr()));
let cli_defaults = load_settings_with_storage_dir(None)?;
let ctx = base_ctx.with_target(&args.target)?;
let created_run = Box::pin(create::create_run(
&ctx,
&args,
cli_defaults,
styles,
true,
printer,
))
.await?;
let created_run = Box::pin(create::create_run(&ctx, &args, styles, true)).await?;
if ctx.user_settings().cli.output.format == OutputFormat::Json {
print_json_pretty(&serde_json::json!({ "run_id": created_run.run_id }))?;
} else {

19
lib/crates/fabro-cli/src/main.rs
View file

@ -166,18 +166,19 @@ async fn main_inner() -> (String, Result<()>) {
Err(err) => return (command_name, Err(err)),
};
let user_settings = match user_config::load_settings() {
let disk_settings = match user_config::load_settings() {
Ok(settings) => settings,
Err(err) => return (command_name, Err(err)),
};
let combined_settings = combine_files(user_settings, SettingsLayer {
let machine_settings = combine_files(disk_settings, SettingsLayer {
cli: Some(cli_layer.clone()),
..SettingsLayer::default()
});
let cli_settings = match fabro_config::UserSettings::from_layer(&combined_settings) {
Ok(settings) => settings.cli,
let user_settings = match fabro_config::UserSettings::from_layer(&machine_settings) {
Ok(settings) => settings,
Err(err) => return (command_name, Err(err.into())),
};
let cli_settings = user_settings.cli.clone();
let printer = printer_from_verbosity(cli_settings.output.verbosity);
let config_log_level = match &pre_tracing_bootstrap.sink {
@ -211,7 +212,15 @@ async fn main_inner() -> (String, Result<()>) {
};
let result = Box::pin(async move {
let build_base_ctx = || CommandContext::base(printer, &cli_layer, process_local_json);
let build_base_ctx = || {
CommandContext::base_with_settings(
printer,
&cli_layer,
process_local_json,
machine_settings.clone(),
user_settings.clone(),
)
};
match *command {
Commands::Exec(args) => commands::exec::execute(args, &cli_settings, printer).await?,

24
lib/crates/fabro-cli/tests/it/cmd/json_global.rs
View file

@ -48,6 +48,30 @@ fn settings_uses_json_output_format_from_home_config() {
assert!(value.is_object());
}
#[test]
fn auth_status_ignores_json_output_format_from_home_config() {
let context = test_context!();
context.write_home(
".fabro/settings.toml",
"_version = 1\n\n[cli.output]\nformat = \"json\"\n",
);
let output = context
.command()
.args(["auth", "status"])
.output()
.expect("command should run");
assert!(output.status.success());
assert!(
output.stdout.is_empty(),
"stdout should stay empty in text mode"
);
let stderr = output_stderr(&output);
assert!(stderr.contains("Not logged in to any servers."));
assert!(stderr.contains("Dev token:"));
}
#[test]
fn secret_list_uses_json_output_format_from_home_config() {
let context = test_context!();

85
lib/crates/fabro-cli/tests/it/cmd/system_events.rs
View file

@ -1,4 +1,5 @@
use fabro_test::{fabro_snapshot, test_context};
use httpmock::MockServer;
#[test]
fn help() {
@ -26,3 +27,87 @@ fn help() {
----- stderr -----
");
}
#[test]
fn system_events_renders_text_lines_from_sse_payloads() {
let context = test_context!();
let server = MockServer::start();
let run_id = crate::support::unique_run_id();
let payload = serde_json::json!({
"payload": {
"ts": "2026-04-05T12:00:00Z",
"run_id": run_id,
"event": "run.completed",
}
});
let attach_mock = server.mock(|when, then| {
when.method("GET")
.path("/api/v1/attach")
.query_param("run_id", run_id.as_str());
then.status(200)
.header("Content-Type", "text/event-stream")
.body(format!("data: {payload}\n\n"));
});
let output = context
.command()
.args([
"system",
"events",
"--server",
&format!("{}/api/v1", server.base_url()),
"--run-id",
&run_id,
])
.output()
.expect("command should run");
assert!(output.status.success(), "system events failed");
let stdout = String::from_utf8(output.stdout).expect("stdout should be UTF-8");
assert_eq!(
stdout.trim(),
format!("2026-04-05T12:00:00Z {} run.completed", &run_id[..12])
);
attach_mock.assert();
}
#[test]
fn system_events_json_emits_raw_sse_payloads() {
let context = test_context!();
let server = MockServer::start();
let run_id = crate::support::unique_run_id();
let payload = serde_json::json!({
"payload": {
"ts": "2026-04-05T12:00:00Z",
"run_id": run_id,
"event": "run.completed",
}
});
let attach_mock = server.mock(|when, then| {
when.method("GET")
.path("/api/v1/attach")
.query_param("run_id", run_id.as_str());
then.status(200)
.header("Content-Type", "text/event-stream")
.body(format!("data: {payload}\n\n"));
});
let output = context
.command()
.args([
"--json",
"system",
"events",
"--server",
&format!("{}/api/v1", server.base_url()),
"--run-id",
&run_id,
])
.output()
.expect("command should run");
assert!(output.status.success(), "system events failed");
let stdout = String::from_utf8(output.stdout).expect("stdout should be UTF-8");
assert_eq!(stdout.trim(), payload.to_string());
attach_mock.assert();
}

38
lib/crates/fabro-cli/tests/it/cmd/system_info.rs
View file

@ -48,3 +48,41 @@ fn system_info_json_reports_runtime_fields() {
assert!(value["uptime_secs"].is_number());
assert!(value["runs"]["total"].is_number());
}
#[test]
fn system_info_uses_explicit_storage_dir_override() {
let mut context = test_context!();
let storage_dir = context.temp_dir.join("alternate-storage");
std::fs::create_dir_all(&storage_dir).unwrap();
context.write_home(
".fabro/settings.toml",
format!(
"_version = 1\n\n[server.storage]\nroot = {:?}\n",
context.storage_dir.display().to_string()
),
);
context.ensure_home_server_auth_methods();
context.manage_storage_dir(&storage_dir);
let output = context
.command()
.args([
"--json",
"system",
"info",
"--storage-dir",
storage_dir.to_str().unwrap(),
])
.output()
.expect("command should run");
assert!(
output.status.success(),
"system info failed\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
let value: Value =
serde_json::from_slice(&output.stdout).expect("system info JSON should parse");
assert_eq!(value["storage_dir"], storage_dir.display().to_string());
}