From 7e5c343ef5cfccafcfda8dcca8c04869da97affe Mon Sep 17 00:00:00 2001
From: Michael Bolin <mbolin@openai.com>
Date: Fri, 19 Dec 2025 11:03:50 -0800
Subject: [PATCH] feat: make ConstraintError an enum (#8330)

This will make it easier to test for expected errors in unit tests since
we can compare based on the field values rather than the message (which
might change over time). See https://github.com/openai/codex/pull/8298
for an example.

It also ensures more consistency in the way a `ConstraintError` is
constructed.
---
 codex-rs/core/src/codex.rs             | 15 ++++-----------
 codex-rs/core/src/config/constraint.rs | 22 +++++++++++-----------
 codex-rs/tui/src/chatwidget/tests.rs   | 16 ++++++++--------
 3 files changed, 23 insertions(+), 30 deletions(-)

diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs
index f0d205658..440135f7f 100644
--- a/codex-rs/core/src/codex.rs
+++ b/codex-rs/core/src/codex.rs
@@ -78,7 +78,6 @@ use crate::client_common::ResponseEvent;
 use crate::compact::collect_user_messages;
 use crate::config::Config;
 use crate::config::Constrained;
-use crate::config::ConstraintError;
 use crate::config::ConstraintResult;
 use crate::config::GhostSnapshotConfig;
 use crate::config::types::ShellEnvironmentPolicy;
@@ -836,11 +835,8 @@ impl Session {
                 Ok(())
             }
             Err(err) => {
-                let wrapped = ConstraintError {
-                    message: format!("Could not update config: {err}"),
-                };
-                warn!(%wrapped, "rejected session settings update");
-                Err(wrapped)
+                warn!("rejected session settings update: {err}");
+                Err(err)
             }
         }
     }
@@ -861,18 +857,15 @@ impl Session {
                 }
                 Err(err) => {
                     drop(state);
-                    let wrapped = ConstraintError {
-                        message: format!("Could not update config: {err}"),
-                    };
                     self.send_event_raw(Event {
                         id: sub_id.clone(),
                         msg: EventMsg::Error(ErrorEvent {
-                            message: wrapped.to_string(),
+                            message: err.to_string(),
                             codex_error_info: Some(CodexErrorInfo::BadRequest),
                         }),
                     })
                     .await;
-                    return Err(wrapped);
+                    return Err(err);
                 }
             }
         };
diff --git a/codex-rs/core/src/config/constraint.rs b/codex-rs/core/src/config/constraint.rs
index d126b84a8..795a8d568 100644
--- a/codex-rs/core/src/config/constraint.rs
+++ b/codex-rs/core/src/config/constraint.rs
@@ -4,25 +4,25 @@ use std::sync::Arc;
 use thiserror::Error;
 
 #[derive(Debug, Error, PartialEq, Eq)]
-#[error("{message}")]
-pub struct ConstraintError {
-    pub message: String,
+pub enum ConstraintError {
+    #[error("value `{candidate}` is not in the allowed set {allowed}")]
+    InvalidValue { candidate: String, allowed: String },
+
+    #[error("field `{field_name}` cannot be empty")]
+    EmptyField { field_name: String },
 }
 
 impl ConstraintError {
     pub fn invalid_value(candidate: impl Into<String>, allowed: impl Into<String>) -> Self {
-        Self {
-            message: format!(
-                "value `{}` is not in the allowed set {}",
-                candidate.into(),
-                allowed.into()
-            ),
+        Self::InvalidValue {
+            candidate: candidate.into(),
+            allowed: allowed.into(),
         }
     }
 
     pub fn empty_field(field_name: impl Into<String>) -> Self {
-        Self {
-            message: format!("field `{}` cannot be empty", field_name.into()),
+        Self::EmptyField {
+            field_name: field_name.into(),
         }
     }
 }
diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs
index fe96b5f97..344208f73 100644
--- a/codex-rs/tui/src/chatwidget/tests.rs
+++ b/codex-rs/tui/src/chatwidget/tests.rs
@@ -2275,12 +2275,12 @@ async fn approvals_popup_shows_disabled_presets() {
     chat.config.approval_policy =
         Constrained::new(AskForApproval::OnRequest, |candidate| match candidate {
             AskForApproval::OnRequest => Ok(()),
-            _ => Err(ConstraintError {
-                message: "this message should be printed in the description".to_string(),
-            }),
+            _ => Err(ConstraintError::invalid_value(
+                candidate.to_string(),
+                "this message should be printed in the description",
+            )),
         })
         .expect("construct constrained approval policy");
-
     chat.open_approvals_popup();
 
     let width = 80;
@@ -2311,12 +2311,12 @@ async fn approvals_popup_navigation_skips_disabled() {
     chat.config.approval_policy =
         Constrained::new(AskForApproval::OnRequest, |candidate| match candidate {
             AskForApproval::OnRequest => Ok(()),
-            _ => Err(ConstraintError {
-                message: "disabled preset".to_string(),
-            }),
+            _ => Err(ConstraintError::invalid_value(
+                candidate.to_string(),
+                "[on-request]",
+            )),
         })
         .expect("construct constrained approval policy");
-
     chat.open_approvals_popup();
 
     // The approvals popup is the active bottom-pane view; drive navigation via chat handle_key_event.