Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions codex-rs/core/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1588,9 +1588,9 @@ impl Config {
model_provider_id,
model_provider,
cwd: resolved_cwd,
approval_policy: constrained_approval_policy,
sandbox_policy: constrained_sandbox_policy,
enforce_residency,
approval_policy: constrained_approval_policy.value,
sandbox_policy: constrained_sandbox_policy.value,
enforce_residency: enforce_residency.value,
did_user_set_custom_approval_policy_or_sandbox_mode,
forced_auto_mode_downgraded_on_windows,
shell_environment_policy,
Expand Down
110 changes: 89 additions & 21 deletions codex-rs/core/src/config_loader/config_requirements.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,25 +44,57 @@ impl fmt::Display for RequirementSource {
}
}

#[derive(Debug, Clone, PartialEq)]
pub struct ConstrainedWithSource<T> {
pub value: Constrained<T>,
pub source: Option<RequirementSource>,
}

impl<T> ConstrainedWithSource<T> {
pub fn new(value: Constrained<T>, source: Option<RequirementSource>) -> Self {
Self { value, source }
}
}

impl<T> std::ops::Deref for ConstrainedWithSource<T> {
type Target = Constrained<T>;

fn deref(&self) -> &Self::Target {
&self.value
}
}

impl<T> std::ops::DerefMut for ConstrainedWithSource<T> {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.value
}
}

/// Normalized version of [`ConfigRequirementsToml`] after deserialization and
/// normalization.
#[derive(Debug, Clone, PartialEq)]
pub struct ConfigRequirements {
pub approval_policy: Constrained<AskForApproval>,
pub sandbox_policy: Constrained<SandboxPolicy>,
pub approval_policy: ConstrainedWithSource<AskForApproval>,
pub sandbox_policy: ConstrainedWithSource<SandboxPolicy>,
pub mcp_servers: Option<Sourced<BTreeMap<String, McpServerRequirement>>>,
pub(crate) exec_policy: Option<Sourced<RequirementsExecPolicy>>,
pub enforce_residency: Constrained<Option<ResidencyRequirement>>,
pub enforce_residency: ConstrainedWithSource<Option<ResidencyRequirement>>,
}

impl Default for ConfigRequirements {
fn default() -> Self {
Self {
approval_policy: Constrained::allow_any_from_default(),
sandbox_policy: Constrained::allow_any(SandboxPolicy::ReadOnly),
approval_policy: ConstrainedWithSource::new(
Constrained::allow_any_from_default(),
None,
),
sandbox_policy: ConstrainedWithSource::new(
Constrained::allow_any(SandboxPolicy::ReadOnly),
None,
),
mcp_servers: None,
exec_policy: None,
enforce_residency: Constrained::allow_any(None),
enforce_residency: ConstrainedWithSource::new(Constrained::allow_any(None), None),
}
}
}
Expand Down Expand Up @@ -228,7 +260,7 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
enforce_residency,
} = toml;

let approval_policy: Constrained<AskForApproval> = match allowed_approval_policies {
let approval_policy = match allowed_approval_policies {
Some(Sourced {
value: policies,
source: requirement_source,
Expand All @@ -237,20 +269,22 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
return Err(ConstraintError::empty_field("allowed_approval_policies"));
};

Constrained::new(initial_value, move |candidate| {
let requirement_source_for_error = requirement_source.clone();
let constrained = Constrained::new(initial_value, move |candidate| {
if policies.contains(candidate) {
Ok(())
} else {
Err(ConstraintError::InvalidValue {
field_name: "approval_policy",
candidate: format!("{candidate:?}"),
allowed: format!("{policies:?}"),
requirement_source: requirement_source.clone(),
requirement_source: requirement_source_for_error.clone(),
})
}
})?
})?;
ConstrainedWithSource::new(constrained, Some(requirement_source))
}
None => Constrained::allow_any_from_default(),
None => ConstrainedWithSource::new(Constrained::allow_any_from_default(), None),
};

// TODO(gt): `ConfigRequirementsToml` should let the author specify the
Expand All @@ -261,7 +295,7 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
// additional parameters. Ultimately, we should expand the config
// format to allow specifying those parameters.
let default_sandbox_policy = SandboxPolicy::ReadOnly;
let sandbox_policy: Constrained<SandboxPolicy> = match allowed_sandbox_modes {
let sandbox_policy = match allowed_sandbox_modes {
Some(Sourced {
value: modes,
source: requirement_source,
Expand All @@ -275,7 +309,8 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
});
};

Constrained::new(default_sandbox_policy, move |candidate| {
let requirement_source_for_error = requirement_source.clone();
let constrained = Constrained::new(default_sandbox_policy, move |candidate| {
let mode = match candidate {
SandboxPolicy::ReadOnly => SandboxModeRequirement::ReadOnly,
SandboxPolicy::WorkspaceWrite { .. } => {
Expand All @@ -293,12 +328,15 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
field_name: "sandbox_mode",
candidate: format!("{mode:?}"),
allowed: format!("{modes:?}"),
requirement_source: requirement_source.clone(),
requirement_source: requirement_source_for_error.clone(),
})
}
})?
})?;
ConstrainedWithSource::new(constrained, Some(requirement_source))
}
None => {
ConstrainedWithSource::new(Constrained::allow_any(default_sandbox_policy), None)
}
None => Constrained::allow_any(default_sandbox_policy),
};
let exec_policy = match rules {
Some(Sourced { value, source }) => {
Expand All @@ -313,26 +351,28 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
None => None,
};

let enforce_residency: Constrained<Option<ResidencyRequirement>> = match enforce_residency {
let enforce_residency = match enforce_residency {
Some(Sourced {
value: residency,
source: requirement_source,
}) => {
let required = Some(residency);
Constrained::new(required, move |candidate| {
let requirement_source_for_error = requirement_source.clone();
let constrained = Constrained::new(required, move |candidate| {
if candidate == &required {
Ok(())
} else {
Err(ConstraintError::InvalidValue {
field_name: "enforce_residency",
candidate: format!("{candidate:?}"),
allowed: format!("{required:?}"),
requirement_source: requirement_source.clone(),
requirement_source: requirement_source_for_error.clone(),
})
}
})?
})?;
ConstrainedWithSource::new(constrained, Some(requirement_source))
}
None => Constrained::allow_any(None),
None => ConstrainedWithSource::new(Constrained::allow_any(None), None),
};
Ok(ConfigRequirements {
approval_policy,
Expand Down Expand Up @@ -563,6 +603,34 @@ mod tests {
Ok(())
}

#[test]
fn constrained_fields_store_requirement_source() -> Result<()> {
let source: ConfigRequirementsToml = from_str(
r#"
allowed_approval_policies = ["on-request"]
allowed_sandbox_modes = ["read-only"]
enforce_residency = "us"
"#,
)?;

let source_location = RequirementSource::CloudRequirements;
let mut target = ConfigRequirementsWithSources::default();
target.merge_unset_fields(source_location.clone(), source);
let requirements = ConfigRequirements::try_from(target)?;

assert_eq!(
requirements.approval_policy.source,
Some(source_location.clone())
);
assert_eq!(
requirements.sandbox_policy.source,
Some(source_location.clone())
);
assert_eq!(requirements.enforce_residency.source, Some(source_location));

Ok(())
}

#[test]
fn deserialize_allowed_approval_policies() -> Result<()> {
let toml_str = r#"
Expand Down
1 change: 1 addition & 0 deletions codex-rs/core/src/config_loader/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use toml::Value as TomlValue;
pub use cloud_requirements::CloudRequirementsLoader;
pub use config_requirements::ConfigRequirements;
pub use config_requirements::ConfigRequirementsToml;
pub use config_requirements::ConstrainedWithSource;
pub use config_requirements::McpServerIdentity;
pub use config_requirements::McpServerRequirement;
pub use config_requirements::RequirementSource;
Expand Down
Loading