diff --git a/lib/crates/fabro-api/build.rs b/lib/crates/fabro-api/build.rs index 0e5de32ce..b63e23029 100644 --- a/lib/crates/fabro-api/build.rs +++ b/lib/crates/fabro-api/build.rs @@ -379,16 +379,6 @@ fn main() { ("PreRunPushOutcome", "fabro_types::PreRunPushOutcome", &[]), ("DirtyStatus", "fabro_types::DirtyStatus", &[]), ("GitContext", "fabro_types::GitContext", &[]), - ( - "RunIntegrationsSettings", - "fabro_types::settings::run::RunIntegrationsSettings", - &[], - ), - ( - "RunIntegrationsGithubSettings", - "fabro_types::settings::run::RunIntegrationsGithubSettings", - &[], - ), ]; for (name, path, impls) in replacements { settings.with_replacement(*name, *path, impls.iter().copied()); diff --git a/lib/crates/fabro-api/tests/run_integrations_round_trip.rs b/lib/crates/fabro-api/tests/run_integrations_round_trip.rs new file mode 100644 index 000000000..243268f1c --- /dev/null +++ b/lib/crates/fabro-api/tests/run_integrations_round_trip.rs @@ -0,0 +1,64 @@ +//! JSON parity test for `RunIntegrationsGithubSettings`. +//! +//! Asserts that the API-side generated `RunIntegrationsGithubSettings` and +//! the canonical Rust resolved type round-trip through the same JSON shape. +//! Covers both the populated and empty-permissions cases. + +use fabro_api::types::{ + RunIntegrationsGithubSettings as ApiRunIntegrationsGithubSettings, + RunIntegrationsSettings as ApiRunIntegrationsSettings, +}; +use fabro_types::settings::run::{RunIntegrationsGithubSettings, RunIntegrationsSettings}; +use serde_json::json; + +#[test] +fn run_integrations_github_settings_round_trips_with_permissions() { + let json_value = json!({ + "permissions": { + "issues": "read", + "contents": "write", + } + }); + + let api: ApiRunIntegrationsGithubSettings = + serde_json::from_value(json_value.clone()).expect("api type should parse"); + let canonical: RunIntegrationsGithubSettings = + serde_json::from_value(json_value.clone()).expect("canonical type should parse"); + + assert_eq!(serde_json::to_value(&api).unwrap(), json_value); + assert_eq!(serde_json::to_value(&canonical).unwrap(), json_value); +} + +#[test] +fn run_integrations_github_settings_round_trips_empty_permissions() { + // Empty map is the resolved form of "no token requested" — must + // serialize as an object, not omitted. + let json_value = json!({ "permissions": {} }); + + let api: ApiRunIntegrationsGithubSettings = + serde_json::from_value(json_value.clone()).expect("api type should parse empty"); + let canonical: RunIntegrationsGithubSettings = + serde_json::from_value(json_value.clone()).expect("canonical type should parse empty"); + + assert_eq!(serde_json::to_value(&api).unwrap(), json_value); + assert_eq!(serde_json::to_value(&canonical).unwrap(), json_value); +} + +#[test] +fn run_integrations_settings_round_trips() { + let json_value = json!({ + "github": { + "permissions": { + "issues": "read", + } + } + }); + + let api: ApiRunIntegrationsSettings = + serde_json::from_value(json_value.clone()).expect("api wrapper should parse"); + let canonical: RunIntegrationsSettings = + serde_json::from_value(json_value.clone()).expect("canonical wrapper should parse"); + + assert_eq!(serde_json::to_value(&api).unwrap(), json_value); + assert_eq!(serde_json::to_value(&canonical).unwrap(), json_value); +} diff --git a/lib/crates/fabro-config/src/layers/combine.rs b/lib/crates/fabro-config/src/layers/combine.rs index ce82485a3..58a0f2a03 100644 --- a/lib/crates/fabro-config/src/layers/combine.rs +++ b/lib/crates/fabro-config/src/layers/combine.rs @@ -102,12 +102,6 @@ impl Combine for Option> { } } -impl Combine for Option> { - fn combine(self, other: Self) -> Self { - self.or(other) - } -} - macro_rules! impl_combine_self { ($($ty:ty),+ $(,)?) => { $( diff --git a/lib/crates/fabro-config/src/layers/run.rs b/lib/crates/fabro-config/src/layers/run.rs index 23946d17f..74e53a53d 100644 --- a/lib/crates/fabro-config/src/layers/run.rs +++ b/lib/crates/fabro-config/src/layers/run.rs @@ -9,6 +9,7 @@ use fabro_types::settings::run::{ use fabro_types::settings::{Duration, InterpString, ModelRef, Size}; use serde::{Deserialize, Serialize}; +use super::combine::Combine; use super::maps::{MergeMap, ReplaceMap, StickyMap}; use super::splice_array::SPLICE_MARKER; @@ -65,20 +66,25 @@ pub struct RunIntegrationsLayer { /// `[run.integrations.github]` — runtime GitHub token shape. /// -/// The `permissions` field is `Option>` so a higher layer that -/// sets `permissions = {}` is honored as an explicit clear (no token -/// requested) rather than falling through to a lower layer. `Combine` for -/// `Option>` uses `or` semantics (defined in `combine.rs`), so -/// any `Some(_)` (including `Some({})`) wins over the fallback layer. This -/// diverges from sibling map fields like `RunLayer::metadata` (`ReplaceMap`), -/// where an empty table means inherit. -#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize, fabro_macros::Combine)] +/// `Combine` is hand-rolled (not derived) so any higher-layer `permissions` +/// value fully replaces the fallback, including `Some({})` as an explicit +/// clear. This intentionally differs from `ReplaceMap`, whose empty map falls +/// back to lower layers. +#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] #[serde(deny_unknown_fields)] pub struct RunIntegrationsGithubLayer { #[serde(default, skip_serializing_if = "Option::is_none")] pub permissions: Option>, } +impl Combine for RunIntegrationsGithubLayer { + fn combine(self, other: Self) -> Self { + Self { + permissions: self.permissions.or(other.permissions), + } + } +} + /// The source of a run's goal, either inline literal text or a reference to /// a file on disk. ///