From bd4aa787ca6b8fd7ca55a741bf686d020e9b9f28 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 9 Apr 2026 18:21:59 -0400 Subject: [PATCH] refactor(settings): stage 6.3b promote run runtime types + delete to_runtime MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moves the only actively-used types from `fabro-types/src/settings/run.rs` — `PullRequestSettings`, `MergeStrategy`, `ArtifactsSettings` — into a new `fabro-workflow/src/config.rs` module. The other types in that file (`LlmSettings`, `SetupSettings`, `CheckpointSettings`, `GitHubSettings`) had no remaining consumers in the workspace and are deleted outright. `bridge_pull_request`, `bridge_merge_strategy`, and `bridge_run_artifacts` move along with them into `fabro-workflow/src/config.rs`. That empties `fabro-types/src/settings/v2/to_runtime.rs`, so the file is deleted and its `pub mod` declaration removed from `v2/mod.rs`. Stage 6.2's "narrow runtime-type conversion helpers" module is completely gone. Consumer updates: - `fabro-workflow/src/lib.rs` exposes `pub mod config`. - `fabro-workflow/src/operations/start.rs` imports `PullRequestSettings` and `bridge_pull_request` from `crate::config`. - `fabro-workflow/src/pipeline/types.rs` imports `PullRequestSettings` from `crate::config`. - `fabro-workflow/src/pipeline/pull_request.rs` imports `MergeStrategy` from `crate::config`. `fabro-types/src/settings/mod.rs` drops `pub mod run` and the corresponding `pub use run::{ArtifactsSettings, ...}` re-export. Six of the seven legacy runtime type modules are now gone; only `server.rs` remains. 3,758 workspace tests pass. `cargo fmt --check --all` and `cargo clippy --workspace -- -D warnings` are clean. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/crates/fabro-types/src/settings/mod.rs | 5 -- lib/crates/fabro-types/src/settings/run.rs | 61 ---------------- lib/crates/fabro-types/src/settings/v2/mod.rs | 1 - .../fabro-types/src/settings/v2/to_runtime.rs | 47 ------------ lib/crates/fabro-workflow/src/config.rs | 71 +++++++++++++++++++ lib/crates/fabro-workflow/src/lib.rs | 1 + .../fabro-workflow/src/operations/start.rs | 4 +- .../src/pipeline/pull_request.rs | 2 +- .../fabro-workflow/src/pipeline/types.rs | 2 +- 9 files changed, 76 insertions(+), 118 deletions(-) delete mode 100644 lib/crates/fabro-types/src/settings/run.rs delete mode 100644 lib/crates/fabro-types/src/settings/v2/to_runtime.rs create mode 100644 lib/crates/fabro-workflow/src/config.rs diff --git a/lib/crates/fabro-types/src/settings/mod.rs b/lib/crates/fabro-types/src/settings/mod.rs index ec488758b..de5da8e98 100644 --- a/lib/crates/fabro-types/src/settings/mod.rs +++ b/lib/crates/fabro-types/src/settings/mod.rs @@ -19,14 +19,9 @@ //! owning consumer crates or replace their call sites with v2-native //! accessors, at which point this module goes away. -pub mod run; pub mod server; pub mod v2; -pub use run::{ - ArtifactsSettings, CheckpointSettings, GitHubSettings, LlmSettings, MergeStrategy, - PullRequestSettings, SetupSettings, -}; pub use server::{ ApiAuthStrategy, ApiSettings, ArtifactStorageBackend, ArtifactStorageSettings, AuthProvider, AuthSettings, FeaturesSettings, GitAuthorSettings, GitProvider, GitSettings, LogSettings, diff --git a/lib/crates/fabro-types/src/settings/run.rs b/lib/crates/fabro-types/src/settings/run.rs deleted file mode 100644 index a46ad0b7e..000000000 --- a/lib/crates/fabro-types/src/settings/run.rs +++ /dev/null @@ -1,61 +0,0 @@ -use std::collections::HashMap; - -use serde::{Deserialize, Serialize}; - -#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)] -pub struct CheckpointSettings { - #[serde(default)] - pub exclude_globs: Vec, -} - -fn default_true() -> bool { - true -} - -#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)] -pub struct PullRequestSettings { - #[serde(default)] - pub enabled: bool, - #[serde(default = "default_true")] - pub draft: bool, - #[serde(default)] - pub auto_merge: bool, - #[serde(default)] - pub merge_strategy: MergeStrategy, -} - -#[derive(Clone, Copy, Debug, Default, Deserialize, PartialEq, Serialize, crate::Combine)] -#[serde(rename_all = "lowercase")] -pub enum MergeStrategy { - #[default] - Squash, - Merge, - Rebase, -} - -#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)] -pub struct ArtifactsSettings { - #[serde(default)] - pub include: Vec, -} - -#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)] -pub struct GitHubSettings { - #[serde(default)] - pub permissions: HashMap, -} - -#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)] -pub struct LlmSettings { - pub model: Option, - pub provider: Option, - #[serde(default)] - pub fallbacks: Option>>, -} - -#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)] -pub struct SetupSettings { - #[serde(default)] - pub commands: Vec, - pub timeout_ms: Option, -} diff --git a/lib/crates/fabro-types/src/settings/v2/mod.rs b/lib/crates/fabro-types/src/settings/v2/mod.rs index 9304b4082..8e2e9ddce 100644 --- a/lib/crates/fabro-types/src/settings/v2/mod.rs +++ b/lib/crates/fabro-types/src/settings/v2/mod.rs @@ -17,7 +17,6 @@ pub mod run; pub mod server; pub mod size; pub mod splice_array; -pub mod to_runtime; pub mod tree; pub mod version; pub mod workflow; diff --git a/lib/crates/fabro-types/src/settings/v2/to_runtime.rs b/lib/crates/fabro-types/src/settings/v2/to_runtime.rs deleted file mode 100644 index 192a77862..000000000 --- a/lib/crates/fabro-types/src/settings/v2/to_runtime.rs +++ /dev/null @@ -1,47 +0,0 @@ -//! v2 → runtime-type conversion helpers. -//! -//! Everything but the pull-request / artifacts / merge-strategy conversion -//! has moved out of this module into the consumer crate that owns the -//! target runtime type: -//! -//! - Hook bridging: [`fabro_hooks::config::bridge_hook`] -//! - MCP bridging: [`fabro_mcp::config::bridge_mcp_entry`] / -//! [`fabro_mcp::config::bridge_mcps`] -//! - Sandbox bridging: [`fabro_sandbox::config::bridge_sandbox`] / -//! [`fabro_sandbox::config::bridge_worktree_mode`] -//! -//! Pull-request / artifacts / merge-strategy still live here because their -//! target runtime types (`PullRequestSettings`, `ArtifactsSettings`, -//! `MergeStrategy`) are still in `fabro-types::settings::run`. When that -//! module moves into `fabro-workflow` the remaining helpers will follow. - -use super::run::{MergeStrategy as V2MergeStrategy, RunArtifactsLayer, RunPullRequestLayer}; -use crate::settings::run::{ - ArtifactsSettings, MergeStrategy as OldMergeStrategy, PullRequestSettings, -}; - -pub fn bridge_merge_strategy(m: V2MergeStrategy) -> OldMergeStrategy { - match m { - V2MergeStrategy::Squash => OldMergeStrategy::Squash, - V2MergeStrategy::Merge => OldMergeStrategy::Merge, - V2MergeStrategy::Rebase => OldMergeStrategy::Rebase, - } -} - -pub fn bridge_pull_request(pr: &RunPullRequestLayer) -> PullRequestSettings { - PullRequestSettings { - enabled: pr.enabled.unwrap_or(false), - draft: pr.draft.unwrap_or(true), - auto_merge: pr.auto_merge.unwrap_or(false), - merge_strategy: pr - .merge_strategy - .map(bridge_merge_strategy) - .unwrap_or_default(), - } -} - -pub fn bridge_run_artifacts(artifacts: &RunArtifactsLayer) -> ArtifactsSettings { - ArtifactsSettings { - include: artifacts.include.clone(), - } -} diff --git a/lib/crates/fabro-workflow/src/config.rs b/lib/crates/fabro-workflow/src/config.rs new file mode 100644 index 000000000..aba443698 --- /dev/null +++ b/lib/crates/fabro-workflow/src/config.rs @@ -0,0 +1,71 @@ +//! Workflow runtime configuration shapes. +//! +//! Runtime-side types consumed by the pipeline. The v2 parse tree lives in +//! `fabro_types::settings::v2::run::{RunPullRequestLayer, MergeStrategy, +//! RunArtifactsLayer}`. Conversion from v2 lives in [`bridge_pull_request`] +//! / [`bridge_run_artifacts`]. + +use fabro_types::settings::v2::run::{ + MergeStrategy as V2MergeStrategy, RunArtifactsLayer, RunPullRequestLayer, +}; +use serde::{Deserialize, Serialize}; + +fn default_true() -> bool { + true +} + +#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)] +pub struct PullRequestSettings { + #[serde(default)] + pub enabled: bool, + #[serde(default = "default_true")] + pub draft: bool, + #[serde(default)] + pub auto_merge: bool, + #[serde(default)] + pub merge_strategy: MergeStrategy, +} + +#[derive(Clone, Copy, Debug, Default, Deserialize, PartialEq, Serialize)] +#[serde(rename_all = "lowercase")] +pub enum MergeStrategy { + #[default] + Squash, + Merge, + Rebase, +} + +#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)] +pub struct ArtifactsSettings { + #[serde(default)] + pub include: Vec, +} + +#[must_use] +pub fn bridge_merge_strategy(m: V2MergeStrategy) -> MergeStrategy { + match m { + V2MergeStrategy::Squash => MergeStrategy::Squash, + V2MergeStrategy::Merge => MergeStrategy::Merge, + V2MergeStrategy::Rebase => MergeStrategy::Rebase, + } +} + +#[must_use] +pub fn bridge_pull_request(pr: &RunPullRequestLayer) -> PullRequestSettings { + PullRequestSettings { + enabled: pr.enabled.unwrap_or(false), + draft: pr.draft.unwrap_or(true), + auto_merge: pr.auto_merge.unwrap_or(false), + merge_strategy: pr + .merge_strategy + .map(bridge_merge_strategy) + .unwrap_or_default(), + } +} + +#[must_use] +pub fn bridge_run_artifacts(artifacts: &RunArtifactsLayer) -> ArtifactsSettings { + ArtifactsSettings { + include: artifacts.include.clone(), + } +} diff --git a/lib/crates/fabro-workflow/src/lib.rs b/lib/crates/fabro-workflow/src/lib.rs index 76e2311a1..270ce6d9d 100644 --- a/lib/crates/fabro-workflow/src/lib.rs +++ b/lib/crates/fabro-workflow/src/lib.rs @@ -116,6 +116,7 @@ pub mod artifact; pub mod artifact_snapshot; pub mod artifact_upload; pub(crate) mod condition; +pub mod config; pub mod context; pub mod devcontainer_bridge; pub mod error; diff --git a/lib/crates/fabro-workflow/src/operations/start.rs b/lib/crates/fabro-workflow/src/operations/start.rs index 55a2c140e..9d120e679 100644 --- a/lib/crates/fabro-workflow/src/operations/start.rs +++ b/lib/crates/fabro-workflow/src/operations/start.rs @@ -15,9 +15,10 @@ use fabro_sandbox::config::{ use fabro_sandbox::{SandboxProvider, SandboxSpec}; use fabro_types::RunId; use fabro_types::settings::v2::run::ModelRefOrSplice; -use fabro_types::settings::v2::to_runtime::bridge_pull_request; use fabro_types::settings::v2::{InterpString, SettingsFile}; +use crate::config::{PullRequestSettings, bridge_pull_request}; + use crate::artifact_upload::ArtifactSink; use crate::context::Context; use crate::error::FabroError; @@ -41,7 +42,6 @@ use crate::workflow_bundle::{RunDefinition, WorkflowBundle}; use fabro_retro::retro::Retro; use fabro_sandbox::daytona::DaytonaConfig; use fabro_sandbox::daytona::detect_repo_info; -use fabro_types::settings::run::PullRequestSettings; use tokio::runtime::Handle; struct RunSession { diff --git a/lib/crates/fabro-workflow/src/pipeline/pull_request.rs b/lib/crates/fabro-workflow/src/pipeline/pull_request.rs index 21eb8d84d..97ed9aaff 100644 --- a/lib/crates/fabro-workflow/src/pipeline/pull_request.rs +++ b/lib/crates/fabro-workflow/src/pipeline/pull_request.rs @@ -1,6 +1,6 @@ +use crate::config::MergeStrategy; use fabro_store::RunProjection; use fabro_types::PullRequestRecord; -use fabro_types::settings::run::MergeStrategy; use tracing::{debug, info}; use fabro_github::{self as github_app, GitHubAppCredentials, ssh_url_to_https}; diff --git a/lib/crates/fabro-workflow/src/pipeline/types.rs b/lib/crates/fabro-workflow/src/pipeline/types.rs index 883093f28..653a9fc12 100644 --- a/lib/crates/fabro-workflow/src/pipeline/types.rs +++ b/lib/crates/fabro-workflow/src/pipeline/types.rs @@ -15,6 +15,7 @@ use fabro_types::RunId; use fabro_validate::Diagnostic; use crate::artifact_upload::ArtifactSink; +use crate::config::PullRequestSettings; use crate::context::Context; use crate::error::FabroError; use crate::event::Emitter; @@ -29,7 +30,6 @@ use crate::transforms::Transform; use crate::workflow_bundle::WorkflowBundle; use fabro_llm::client::Client; use fabro_retro::retro::Retro; -use fabro_types::settings::run::PullRequestSettings; use fabro_validate::Severity; /// Output of the PARSE phase.