openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-09-15 12:08:01 +00:00
Code Issues Packages Projects Releases Wiki Activity

Add feedback flag to requirements

Browse Source
This commit is contained in:
gt-oai
2026-02-05 13:03:11 +00:00
parent aa46b5cf99
commit 338374f21c
6 changed files with 165 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
codex-rs/app-server/src/config_api.rs
View File

@@ -157,6 +157,7 @@ mod tests {
mcp_servers: None,
rules: None,
enforce_residency: Some(CoreResidencyRequirement::Us),
feedback_enabled: None,
};
let mapped = map_requirements_toml_to_api(requirements);

3
codex-rs/cloud-requirements/src/lib.rs
View File

@@ -384,6 +384,7 @@ mod tests {
mcp_servers: None,
rules: None,
enforce_residency: None,
feedback_enabled: None,
})
);
}
@@ -424,6 +425,7 @@ mod tests {
mcp_servers: None,
rules: None,
enforce_residency: None,
feedback_enabled: None,
})
);
}
@@ -467,6 +469,7 @@ mod tests {
mcp_servers: None,
rules: None,
enforce_residency: None,
feedback_enabled: None,
})
);
assert_eq!(fetcher.request_count.load(Ordering::SeqCst), 2);

80
codex-rs/core/src/config/mod.rs
View File

@@ -1585,6 +1585,7 @@ impl Config {
mcp_servers,
exec_policy: _,
enforce_residency,
feedback_enabled: mut constrained_feedback_enabled,
} = requirements;
constrained_approval_policy
@@ -1593,6 +1594,28 @@ impl Config {
constrained_sandbox_policy
.set(sandbox_policy)
.map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidInput, format!("{e}")))?;
let feedback_enabled_was_explicit = cfg
.feedback
.as_ref()
.and_then(|feedback| feedback.enabled)
.is_some();
let mut feedback_enabled = cfg
.feedback
.as_ref()
.and_then(|feedback| feedback.enabled)
.unwrap_or(true);
if !feedback_enabled_was_explicit
&& let Err(err) = constrained_feedback_enabled.can_set(&feedback_enabled)
{
tracing::warn!(
error = %err,
"default feedback.enabled is disallowed by requirements; falling back to required default"
);
feedback_enabled = constrained_feedback_enabled.value();
}
constrained_feedback_enabled
.set(feedback_enabled)
.map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidInput, format!("{e}")))?;
let mcp_servers = constrain_mcp_servers(cfg.mcp_servers.clone(), mcp_servers.as_ref())
.map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidInput, format!("{e}")))?;
@@ -1689,11 +1712,7 @@ impl Config {
.as_ref()
.and_then(|a| a.enabled)
.or(cfg.analytics.as_ref().and_then(|a| a.enabled)),
feedback_enabled: cfg
.feedback
.as_ref()
.and_then(|feedback| feedback.enabled)
.unwrap_or(true),
feedback_enabled: constrained_feedback_enabled.value(),
tui_notifications: cfg
.tui
.as_ref()
@@ -4701,6 +4720,7 @@ mcp_oauth_callback_port = 5678
mcp_servers: None,
rules: None,
enforce_residency: None,
feedback_enabled: None,
};
let err = ConfigBuilder::default()
@@ -4779,6 +4799,56 @@ trust_level = "untrusted"
assert!(message.contains("set by cloud requirements"));
Ok(())
}
#[tokio::test]
async fn requirements_disallowing_default_feedback_falls_back_to_required_default()
-> std::io::Result<()> {
let codex_home = TempDir::new()?;
let config = ConfigBuilder::default()
.codex_home(codex_home.path().to_path_buf())
.cloud_requirements(CloudRequirementsLoader::new(async {
Some(crate::config_loader::ConfigRequirementsToml {
feedback_enabled: Some(false),
..Default::default()
})
}))
.build()
.await?;
assert!(!config.feedback_enabled);
Ok(())
}
#[tokio::test]
async fn explicit_feedback_enabled_still_errors_when_disallowed_by_requirements()
-> std::io::Result<()> {
let codex_home = TempDir::new()?;
std::fs::write(
codex_home.path().join(CONFIG_TOML_FILE),
r#"[feedback]
enabled = true
"#,
)?;
let err = ConfigBuilder::default()
.codex_home(codex_home.path().to_path_buf())
.fallback_cwd(Some(codex_home.path().to_path_buf()))
.cloud_requirements(CloudRequirementsLoader::new(async {
Some(crate::config_loader::ConfigRequirementsToml {
feedback_enabled: Some(false),
..Default::default()
})
}))
.build()
.await
.expect_err("explicit feedback.enabled=true should fail");
assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput);
let message = err.to_string();
assert!(message.contains("invalid value for `feedback.enabled`"));
assert!(message.contains("set by cloud requirements"));
Ok(())
}
}
#[cfg(test)]

69
codex-rs/core/src/config_loader/config_requirements.rs
View File

@@ -79,6 +79,7 @@ pub struct ConfigRequirements {
pub mcp_servers: Option<Sourced<BTreeMap<String, McpServerRequirement>>>,
pub(crate) exec_policy: Option<Sourced<RequirementsExecPolicy>>,
pub enforce_residency: ConstrainedWithSource<Option<ResidencyRequirement>>,
pub feedback_enabled: ConstrainedWithSource<bool>,
}
impl Default for ConfigRequirements {
@@ -95,6 +96,7 @@ impl Default for ConfigRequirements {
mcp_servers: None,
exec_policy: None,
enforce_residency: ConstrainedWithSource::new(Constrained::allow_any(None), None),
feedback_enabled: ConstrainedWithSource::new(Constrained::allow_any(true), None),
}
}
}
@@ -125,6 +127,7 @@ pub struct ConfigRequirementsToml {
pub mcp_servers: Option<BTreeMap<String, McpServerRequirement>>,
pub rules: Option<RequirementsExecPolicyToml>,
pub enforce_residency: Option<ResidencyRequirement>,
pub feedback_enabled: Option<bool>,
}
/// Value paired with the requirement source it came from, for better error
@@ -156,6 +159,7 @@ pub struct ConfigRequirementsWithSources {
pub mcp_servers: Option<Sourced<BTreeMap<String, McpServerRequirement>>>,
pub rules: Option<Sourced<RequirementsExecPolicyToml>>,
pub enforce_residency: Option<Sourced<ResidencyRequirement>>,
pub feedback_enabled: Option<Sourced<bool>>,
}
impl ConfigRequirementsWithSources {
@@ -189,6 +193,7 @@ impl ConfigRequirementsWithSources {
mcp_servers,
rules,
enforce_residency,
feedback_enabled,
}
);
}
@@ -200,6 +205,7 @@ impl ConfigRequirementsWithSources {
mcp_servers,
rules,
enforce_residency,
feedback_enabled,
} = self;
ConfigRequirementsToml {
allowed_approval_policies: allowed_approval_policies.map(|sourced| sourced.value),
@@ -207,6 +213,7 @@ impl ConfigRequirementsWithSources {
mcp_servers: mcp_servers.map(|sourced| sourced.value),
rules: rules.map(|sourced| sourced.value),
enforce_residency: enforce_residency.map(|sourced| sourced.value),
feedback_enabled: feedback_enabled.map(|sourced| sourced.value),
}
}
}
@@ -251,6 +258,7 @@ impl ConfigRequirementsToml {
&& self.mcp_servers.is_none()
&& self.rules.is_none()
&& self.enforce_residency.is_none()
&& self.feedback_enabled.is_none()
}
}
@@ -264,6 +272,7 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
mcp_servers,
rules,
enforce_residency,
feedback_enabled,
} = toml;
let approval_policy = match allowed_approval_policies {
@@ -380,12 +389,35 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
}
None => ConstrainedWithSource::new(Constrained::allow_any(None), None),
};
let feedback_enabled = match feedback_enabled {
Some(Sourced {
value: feedback_enabled,
source: requirement_source,
}) => {
let requirement_source_for_error = requirement_source.clone();
let constrained = Constrained::new(feedback_enabled, move |candidate| {
if candidate == &feedback_enabled {
Ok(())
} else {
Err(ConstraintError::InvalidValue {
field_name: "feedback.enabled",
candidate: format!("{candidate:?}"),
allowed: format!("{feedback_enabled:?}"),
requirement_source: requirement_source_for_error.clone(),
})
}
})?;
ConstrainedWithSource::new(constrained, Some(requirement_source))
}
None => ConstrainedWithSource::new(Constrained::allow_any(true), None),
};
Ok(ConfigRequirements {
approval_policy,
sandbox_policy,
mcp_servers,
exec_policy,
enforce_residency,
feedback_enabled,
})
}
}
@@ -413,6 +445,7 @@ mod tests {
mcp_servers,
rules,
enforce_residency,
feedback_enabled,
} = toml;
ConfigRequirementsWithSources {
allowed_approval_policies: allowed_approval_policies
@@ -423,6 +456,8 @@ mod tests {
rules: rules.map(|value| Sourced::new(value, RequirementSource::Unknown)),
enforce_residency: enforce_residency
.map(|value| Sourced::new(value, RequirementSource::Unknown)),
feedback_enabled: feedback_enabled
.map(|value| Sourced::new(value, RequirementSource::Unknown)),
}
}
@@ -437,7 +472,9 @@ mod tests {
SandboxModeRequirement::DangerFullAccess,
];
let enforce_residency = ResidencyRequirement::Us;
let feedback_enabled = false;
let enforce_source = source.clone();
let feedback_source = source.clone();
// Intentionally constructed without `..Default::default()` so adding a new field to
// `ConfigRequirementsToml` forces this test to be updated.
@@ -447,6 +484,7 @@ mod tests {
mcp_servers: None,
rules: None,
enforce_residency: Some(enforce_residency),
feedback_enabled: Some(feedback_enabled),
};
target.merge_unset_fields(source.clone(), other);
@@ -462,6 +500,7 @@ mod tests {
mcp_servers: None,
rules: None,
enforce_residency: Some(Sourced::new(enforce_residency, enforce_source)),
feedback_enabled: Some(Sourced::new(feedback_enabled, feedback_source)),
}
);
}
@@ -492,6 +531,7 @@ mod tests {
mcp_servers: None,
rules: None,
enforce_residency: None,
feedback_enabled: None,
}
);
Ok(())
@@ -530,6 +570,7 @@ mod tests {
mcp_servers: None,
rules: None,
enforce_residency: None,
feedback_enabled: None,
}
);
Ok(())
@@ -616,6 +657,7 @@ mod tests {
allowed_approval_policies = ["on-request"]
allowed_sandbox_modes = ["read-only"]
enforce_residency = "us"
feedback_enabled = false
"#,
)?;
@@ -633,6 +675,33 @@ mod tests {
Some(source_location.clone())
);
assert_eq!(requirements.enforce_residency.source, Some(source_location));
assert_eq!(
requirements.feedback_enabled.source,
Some(RequirementSource::CloudRequirements)
);
Ok(())
}
#[test]
fn deserialize_feedback_enabled() -> Result<()> {
let toml_str = r#"
feedback_enabled = false
"#;
let config: ConfigRequirementsToml = from_str(toml_str)?;
let requirements: ConfigRequirements = with_unknown_source(config).try_into()?;
assert_eq!(requirements.feedback_enabled.value(), false);
assert_eq!(
requirements.feedback_enabled.can_set(&true),
Err(ConstraintError::InvalidValue {
field_name: "feedback.enabled",
candidate: "true".into(),
allowed: "false".into(),
requirement_source: RequirementSource::Unknown,
})
);
assert!(requirements.feedback_enabled.can_set(&false).is_ok());
Ok(())
}

3
codex-rs/core/src/config_loader/tests.rs
View File

@@ -539,6 +539,7 @@ allowed_approval_policies = ["on-request"]
mcp_servers: None,
rules: None,
enforce_residency: None,
feedback_enabled: None,
})
}),
)
@@ -585,6 +586,7 @@ allowed_approval_policies = ["on-request"]
mcp_servers: None,
rules: None,
enforce_residency: None,
feedback_enabled: None,
},
);
load_requirements_toml(&mut config_requirements_toml, &requirements_file).await?;
@@ -620,6 +622,7 @@ async fn load_config_layers_includes_cloud_requirements() -> anyhow::Result<()>
mcp_servers: None,
rules: None,
enforce_residency: None,
feedback_enabled: None,
};
let expected = requirements.clone();
let cloud_requirements = CloudRequirementsLoader::new(async move { Some(requirements) });

14
codex-rs/tui/src/debug_config.rs
View File

@@ -99,6 +99,14 @@ fn render_debug_config_lines(stack: &ConfigLayerStack) -> Vec<Line<'static>> {
));
}
if let Some(feedback_enabled) = requirements_toml.feedback_enabled {
requirement_lines.push(requirement_line(
"feedback_enabled",
feedback_enabled.to_string(),
requirements.feedback_enabled.source.as_ref(),
));
}
if requirement_lines.is_empty() {
lines.push(" <none>".dim().into());
} else {
@@ -287,6 +295,10 @@ mod tests {
Constrained::allow_any(Some(ResidencyRequirement::Us)),
Some(RequirementSource::CloudRequirements),
);
requirements.feedback_enabled = ConstrainedWithSource::new(
Constrained::allow_any(false),
Some(RequirementSource::CloudRequirements),
);
let requirements_toml = ConfigRequirementsToml {
allowed_approval_policies: Some(vec![AskForApproval::OnRequest]),
@@ -301,6 +313,7 @@ mod tests {
)])),
rules: None,
enforce_residency: Some(ResidencyRequirement::Us),
feedback_enabled: Some(false),
};
let user_file = if cfg!(windows) {
@@ -333,6 +346,7 @@ mod tests {
);
assert!(rendered.contains("mcp_servers: docs (source: MDM managed_config.toml (legacy))"));
assert!(rendered.contains("enforce_residency: us (source: cloud requirements)"));
assert!(rendered.contains("feedback_enabled: false (source: cloud requirements)"));
assert!(!rendered.contains(" - rules:"));
}
}