scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-09-07 08:27:12 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Extract RunDefaults::merge_overlay to replace inline field-by-field merge

Browse source 
The inline merge in run_command() duplicated the structure of
apply_defaults() with shallower (inconsistent) semantics. This extracts
a proper merge_overlay method that deep-merges compound fields (vars,
hooks, mcp_servers, sandbox sub-fields) consistently with apply_defaults.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-03-14 14:23:23 -04:00
parent 465ec0b956
commit 79ef0672d9
2 changed files with 132 additions and 39 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

40
lib/crates/fabro-workflows/src/cli/run.rs
View file

@ -307,49 +307,11 @@ pub async fn run_command(
// Apply project-level config overrides (fabro.toml) on top of CLI defaults.
// Precedence: workflow.toml > fabro.toml > cli.toml/server.toml
//
// We create a temporary WorkflowRunConfig from cli defaults, apply the project
// defaults on top, then extract the merged RunDefaults. This reuses the same
// field-level merge logic that apply_defaults() already implements.
if let Ok(Some((_config_path, project_config))) =
super::project_config::discover_project_config(&std::env::current_dir().unwrap_or_default())
{
tracing::debug!("Applying run defaults from fabro.toml");
let project_defaults = project_config.into_run_defaults();
// Project overrides cli-level defaults. Build a merged RunDefaults
// by applying cli as base, then overriding with project values.
let mut merged = run_defaults.clone();
if project_defaults.work_dir.is_some() {
merged.work_dir = project_defaults.work_dir;
}
if project_defaults.llm.is_some() {
merged.llm = project_defaults.llm;
}
if project_defaults.setup.is_some() {
merged.setup = project_defaults.setup;
}
if project_defaults.sandbox.is_some() {
merged.sandbox = project_defaults.sandbox;
}
if project_defaults.vars.is_some() {
merged.vars = project_defaults.vars;
}
if !project_defaults.checkpoint.exclude_globs.is_empty() {
merged.checkpoint = project_defaults.checkpoint;
}
if project_defaults.pull_request.is_some() {
merged.pull_request = project_defaults.pull_request;
}
if project_defaults.assets.is_some() {
merged.assets = project_defaults.assets;
}
if !project_defaults.hooks.is_empty() {
merged.hooks = project_defaults.hooks;
}
if !project_defaults.mcp_servers.is_empty() {
merged.mcp_servers = project_defaults.mcp_servers;
}
run_defaults = merged;
run_defaults.merge_overlay(project_config.into_run_defaults());
}
// 0. Resolve workflow arg, load run config if TOML, resolve DOT path, apply defaults

131
lib/crates/fabro-workflows/src/cli/run_config.rs
View file

@ -283,6 +283,137 @@ impl WorkflowRunConfig {
}
}
impl RunDefaults {
/// Merge an overlay on top of this base. The overlay takes precedence
/// for simple fields; compound fields (vars, hooks, mcp_servers) are
/// deep-merged with the overlay winning on collision.
///
/// Uses the same deep-merge semantics as `WorkflowRunConfig::apply_defaults`.
pub fn merge_overlay(&mut self, overlay: RunDefaults) {
if overlay.work_dir.is_some() {
self.work_dir = overlay.work_dir;
}
match (&mut self.llm, overlay.llm) {
(Some(base), Some(over)) => {
if over.model.is_some() {
base.model = over.model;
}
if over.provider.is_some() {
base.provider = over.provider;
}
if over.fallbacks.is_some() {
base.fallbacks = over.fallbacks;
}
}
(None, Some(over)) => self.llm = Some(over),
_ => {}
}
match (&mut self.setup, overlay.setup) {
(Some(base), Some(over)) => {
if over.timeout_ms.is_some() {
base.timeout_ms = over.timeout_ms;
}
}
(None, Some(over)) => self.setup = Some(over),
_ => {}
}
match (&mut self.sandbox, overlay.sandbox) {
(Some(base), Some(over)) => {
if over.provider.is_some() {
base.provider = over.provider;
}
if over.preserve.is_some() {
base.preserve = over.preserve;
}
if over.devcontainer.is_some() {
base.devcontainer = over.devcontainer;
}
if over.local.is_some() {
base.local = over.local;
}
match (&mut base.daytona, over.daytona) {
(Some(base_d), Some(over_d)) => {
if over_d.auto_stop_interval.is_some() {
base_d.auto_stop_interval = over_d.auto_stop_interval;
}
if over_d.snapshot.is_some() {
base_d.snapshot = over_d.snapshot;
}
if let Some(over_labels) = over_d.labels {
let mut merged = base_d.labels.take().unwrap_or_default();
merged.extend(over_labels);
base_d.labels = Some(merged);
}
if over_d.network.is_some() {
base_d.network = over_d.network;
}
}
(None, Some(over_d)) => base.daytona = Some(over_d),
_ => {}
}
#[cfg(feature = "exedev")]
match (&mut base.exe, over.exe) {
(Some(base_e), Some(over_e)) => {
if over_e.image.is_some() {
base_e.image = over_e.image;
}
}
(None, Some(over_e)) => base.exe = Some(over_e),
_ => {}
}
if let Some(over_env) = over.env {
let mut merged = base.env.take().unwrap_or_default();
merged.extend(over_env);
base.env = Some(merged);
}
}
(None, Some(over)) => self.sandbox = Some(over),
_ => {}
}
if let Some(overlay_vars) = overlay.vars {
let mut merged = self.vars.take().unwrap_or_default();
merged.extend(overlay_vars);
self.vars = Some(merged);
}
if !overlay.checkpoint.exclude_globs.is_empty() {
self.checkpoint
.exclude_globs
.append(&mut overlay.checkpoint.exclude_globs.clone());
self.checkpoint.exclude_globs.sort();
self.checkpoint.exclude_globs.dedup();
}
if overlay.pull_request.is_some() {
self.pull_request = overlay.pull_request;
}
if overlay.assets.is_some() {
self.assets = overlay.assets;
}
if !overlay.hooks.is_empty() {
let base = crate::hook::HookConfig {
hooks: std::mem::take(&mut self.hooks),
};
let over = crate::hook::HookConfig {
hooks: overlay.hooks,
};
self.hooks = base.merge(over).hooks;
}
if !overlay.mcp_servers.is_empty() {
let mut merged = std::mem::take(&mut self.mcp_servers);
merged.extend(overlay.mcp_servers);
self.mcp_servers = merged;
}
}
}
/// Load and validate a run config from a TOML file.
///
/// The `graph` path in the returned config is resolved relative to the