Warn on invalid config enum values by aibrahim-oai · Pull Request #21111 · openai/codex

aibrahim-oai · 2026-05-05T00:01:55Z

Why

A single invalid enum-valued setting in config.toml should not make the whole assembled config unusable. The rest of the config may still contain valid model, sandbox, profile, tool, hook, or telemetry settings that Codex can safely apply.

Review feedback also pushed the design away from making invalid values representable throughout ConfigToml. The raw invalid state should stay at the config boundary: warn from the TOML that is about to be loaded, then continue handing normal typed config values to runtime consumers.

What Changed

Added a schema-driven invalid enum warning pass in codex-rs/config/src/enum_warnings.rs.
Exposed that pass as invalid_enum_warnings, so call sites use the same vocabulary: detect invalid enum leaves and return advisory warnings.
The warning pass walks a TOML value alongside the generated config schema through properties, additionalProperties, items, $ref, allOf, anyOf, and oneOf.
Startup loading runs the warning pass over the final merged TOML. Config lock replay runs it over the raw [config] subtree before typed lockfile deserialization.
The pass is advisory and non-mutating. It reports invalid enum-like leaves without rewriting the TOML that is later deserialized.
Added narrow field-level fallbacks for enum leaves that warnings can report, including root/profile settings, project trust levels, tool suggestion entry types, MCP tool approvals, tool config, hook handler type, model_providers.*.wire_api, and OTEL exporters/protocols.
Kept broader shape/type validation with Serde. For example, invalid hook variants are dropped after warning, but malformed known hook variants still fail normally.
Kept app-server config writes strict for newly edited enum values only, so stale tolerated enum values do not block unrelated writes.
Kept relative path resolution from hiding invalid raw enum values: only resolved absolute path strings are copied back into the raw TOML shape.
Regenerated codex-rs/core/config.schema.json for the config shape/default updates.

Verification

Added focused coverage for:

schema traversal across refs, composition, arrays, dynamic maps, and tagged unions;
valid sibling oneOf variants and non-string union branches not warning incorrectly;
invalid root, flattened TUI, nested tool, profile, project, tool-suggestion, MCP, hook, model-provider, and OTEL enum values warning without poisoning unrelated config;
hook invalid variant warnings remaining non-blocking while known malformed hook variants stay strict;
config lock replay surfacing invalid enum warnings from the raw lockfile config;
app-server config writes rejecting newly supplied invalid enum values while allowing unrelated edits when stale enum values already exist;
relative-path resolution preserving invalid raw enum values until warning collection;
startup integration emitting an invalid enum warning while valid settings still reach session configuration.

Ran just write-config-schema after the schema-affecting changes. Local test execution is intentionally left to PR CI per repo guidance for online PRs.

Allow config loading to continue when enum-valued settings contain invalid values. Invalid enum entries are removed from the layer before merging and surfaced through startup config warnings, while unrelated valid settings keep loading normally. Co-authored-by: Codex <noreply@openai.com>

Defer invalid enum handling until after config layers are assembled. This keeps layer loading raw, removes invalid enum values from the final effective config, and reports warnings with the dotted field path and invalid value only. Co-authored-by: Codex <noreply@openai.com>

Replace the explicit config enum sanitizer with a generic deserialize retry loop over the assembled TOML. Unknown enum variant errors remove the offending field, append a warning with the field path and invalid value, and retry deserialization. Co-authored-by: Codex <noreply@openai.com>

Store selected config enum fields as Lenient<T> so invalid values remain visible after deserialization and can be reported as startup warnings while valid consumers unwrap at runtime. Remove the older retry-loop sanitizer now that warnings come from the typed config tree. Co-authored-by: Codex <noreply@openai.com>

Cover the full ConfigBuilder load path for invalid enum values so startup warnings are asserted alongside valid config that still applies. Co-authored-by: Codex <noreply@openai.com>

Remove the explicit invalid_enum_warnings tree walk and have Lenient::into_valid append warning messages when invalid values are consumed. Keep higher-level resolvers returning values instead of accepting active profile and warning sink plumbing. Co-authored-by: Codex <noreply@openai.com>

Keep Lenient::into_valid as the silent projection helper and add into_valid_with_warning for runtime config consumption that should emit startup warnings. Co-authored-by: Codex <noreply@openai.com>

Remove the local warning unwrap helpers so runtime config consumption calls Lenient::into_valid_with_warning directly instead of hiding the API behind wrappers. Co-authored-by: Codex <noreply@openai.com>

chatgpt-codex-connector

💡 Codex Review

codex/codex-rs/exec/src/lib.rs

Line 359 in 9bc30cf

config_toml.cli_auth_credentials_store.unwrap_or_default(),

Unwrap the Lenient credential store before use

cli_auth_credentials_store is now Option<Lenient<AuthCredentialsStoreMode>>, but this call still uses unwrap_or_default() as if it were Option<AuthCredentialsStoreMode>. Lenient<T> has no Default, so codex-exec no longer compiles when this path is checked. Convert through into_valid()/warning handling before passing it on.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Update the TUI config consumer to unwrap the lenient CLI auth credential store before passing it to the cloud requirements loader. Co-authored-by: Codex <noreply@openai.com>

Fix the exec cloud requirements path after cli_auth_credentials_store became lenient. Co-authored-by: Codex <noreply@openai.com>

Address clippy and compile failures from wrapping config enums in Lenient. Co-authored-by: Codex <noreply@openai.com>

Update test expectations after invalid enum values no longer fail whole config deserialization. Co-authored-by: Codex <noreply@openai.com>

jif-oai

TBH I don't think this is the good layer to integrate this

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b1c09a4426

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e1d8a67d5d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e3de954486

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-06T22:23:52Z

+    #[serde_as(deserialize_as = "DefaultOnError")]
    pub approval_policy: Option<AskForApproval>,


Route ConfigToml parsing through the lenient layer merge

Adding DefaultOnError here makes every direct try_into::<ConfigToml>() silently drop invalid enum values. Callers like app-server config read/write still deserialize the raw flattened effective_config(), so a user approval_policy = "bogus" over a managed never is returned/validated as unset instead of falling back per layer or being rejected.

Useful? React with 👍 / 👎.

I think this comment is still right now?
I just tried with approval_policy = { sandbox_approval = true } and this silently become None...

The idea is to do best effort of catching warnings. I catch wrong enum values but it would be harder to catch all ignored failures

chatgpt-codex-connector

💡 Codex Review

codex/codex-rs/config/src/loader/mod.rs

Line 908 in 0e3631c

let resolved_value = resolved_table.get(key).unwrap_or(original_value);

Preserve invalid defaulted enum values through path resolution

This prefers the serialized typed value whenever the key exists in resolved. For non-Option enum fields with defaults, e.g. [tui] notification_method = "loudly", lenient parsing drops the bad value, serialization reintroduces "auto", and this line replaces the original invalid value before the warning pass. The bad config is silently accepted with the default and no startup warning/fallback.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-07T00:40:22Z

+            let (sanitized_layer, _typed_layer, layer_warnings) =
+                deserialize_with_enum_warnings(normalized_layer)?;


Avoid validating each layer before effective overrides apply

Deserializing every layer here makes a lower-precedence non-enum error fatal before a higher layer can override it. For example, a system layer with model_context_window = "bad" and a user/session layer with model_context_window = 100000 used to merge to a valid effective config, but now the system layer parse returns an error and aborts startup.

Useful? React with 👍 / 👎.

aibrahim-oai · 2026-05-07T01:29:04Z

@codex review this

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b487b3592d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f366ac3326

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

aibrahim-oai · 2026-05-07T03:23:49Z

@codex review

chatgpt-codex-connector

💡 Codex Review

codex/codex-rs/core/src/config/mod.rs

Lines 1024 to 1029 in a21345c

    
           let mut config = Config::load_config_with_layer_stack( 
        
               LOCAL_FS.as_ref(), 
        
               lock_config_toml, 
        
               harness_overrides, 
        
               codex_home, 
        
               lock_config_layer_stack,

Preserve enum warnings when loading config locks

The normal config is scanned for enum warnings before this branch, but the lockfile config is then loaded through a fresh ConfigLayerStack with no warning scan. Since ConfigLockfileToml contains ConfigToml, the new DefaultOnError handling can silently drop invalid enum values from the lockfile with no startup warning, so locked sessions may ignore bad locked settings without notice.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-07T03:27:25Z

+    if let Some(warning) = enum_value_warnings(value).into_iter().next() {
+        return Err(SerdeError::custom(warning));


Validate only edited enum values on config writes

Because startup now tolerates invalid enum values, a user can have an ignored stale enum in config and still run Codex. This validation scans the whole user/effective config after any edit, so changing an unrelated key is rejected until the stale enum is fixed. The write path comment/intent is to reject newly provided invalid enum values, not block all unrelated writes.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3f55561327

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-07T03:35:13Z

+    ) -> Result<(TomlValue, ConfigToml, Vec<String>), toml::de::Error> {
+        let merged = self.effective_config();
+        let warnings = invalid_enum_warnings(&merged);
+        let typed = merged.clone().try_into::<ConfigToml>()?;


Make every warned enum non-fatal

Fresh evidence after the earlier nested-enum review: projects.<id>.trust_level and tool_suggest.*[].type still deserialize through strict enum fields. The warning pass will report these invalid values, but this try_into::<ConfigToml>() still returns an error, so startup/config-lock replay abort instead of continuing with a warning.

Useful? React with 👍 / 👎.

jif-oai

I think the direction is better but there are stills edge cases (that will multiply in the future) that makes it not reliable...
A nice macro would be amazing here but I know people don't like macros

jif-oai · 2026-05-07T08:32:30Z

+    #[serde_as(deserialize_as = "DefaultOnError")]
    pub approval_policy: Option<AskForApproval>,


I think this comment is still right now?
I just tried with approval_policy = { sandbox_approval = true } and this silently become None...

jif-oai · 2026-05-07T08:33:03Z

+    // This helper returns ConfigToml directly, so enum warnings are intentionally
+    // not surfaced here. The full ConfigBuilder path is responsible for
+    // attaching them to startup warnings.
+    let (_merged_toml, cfg, _enum_warnings) = config_layer_stack


This leaves codex exec on a silent path: it calls this helper directly, so invalid enum values get defaulted but the warning never reaches the user. Should we surface the warnings here too?

it still consumes the startup warnings. try just codex hi with approval_policy = "potato"

jif-oai · 2026-05-07T08:33:41Z

    /// Defaults to `true`.
    #[serde(default, rename = "notifications")]
+    #[serde_as(deserialize_as = "DefaultOnError")]
    pub notifications: Notifications,


Same issue as before. If I do for example notifications = "xxx", this silently default...

Yeah, because it's an enum

jif-oai · 2026-05-07T08:34:50Z

 #[schemars(deny_unknown_fields)]
 pub struct ToolSuggestConfig {
    #[serde(default)]
+    #[serde_as(deserialize_as = "VecSkipError<_>")]


I'm not sure on this one but this seems to silently drop malformed items in the vec. Which is exactly what we don't want right?

Yeah, because it has enum

jif-oai · 2026-05-11T13:04:09Z

+        ),
+        (
+            Some(&TomlValue::String("make-it-so".to_string())),
+            None,


I still don't get why this is not workspace-write? while it is supposed ot be the higher precedence https://github.com/openai/codex/pull/21111/changes#diff-54f9773abe3add13d5213a4465a234a42359e43793dc79d8f4b63bc3ff849418R322

aibrahim-oai requested a review from a team as a code owner May 5, 2026 00:01

aibrahim-oai and others added 3 commits May 5, 2026 03:13

github-actions Bot mentioned this pull request May 5, 2026

📊 AI CLI 工具社区动态日报 2026-05-05 zx0828/big_model_radar#33

Open

aibrahim-oai and others added 4 commits May 5, 2026 03:49

Add config enum warning integration test

c49e231

Cover the full ConfigBuilder load path for invalid enum values so startup warnings are asserted alongside valid config that still applies. Co-authored-by: Codex <noreply@openai.com>

Split silent and warning lenient unwraps

2e1b882

Keep Lenient::into_valid as the silent projection helper and add into_valid_with_warning for runtime config consumption that should emit startup warnings. Co-authored-by: Codex <noreply@openai.com>

Call lenient warning unwraps directly

9bc30cf

Remove the local warning unwrap helpers so runtime config consumption calls Lenient::into_valid_with_warning directly instead of hiding the API behind wrappers. Co-authored-by: Codex <noreply@openai.com>

chatgpt-codex-connector Bot reviewed May 5, 2026

View reviewed changes

aibrahim-oai and others added 4 commits May 5, 2026 04:18

codex: fix CI failure on PR #21111

d595ef0

Update the TUI config consumer to unwrap the lenient CLI auth credential store before passing it to the cloud requirements loader. Co-authored-by: Codex <noreply@openai.com>

codex: unwrap lenient auth store in exec

da01f35

Fix the exec cloud requirements path after cli_auth_credentials_store became lenient. Co-authored-by: Codex <noreply@openai.com>

codex: fix lenient enum CI fallout

4007c4c

Address clippy and compile failures from wrapping config enums in Lenient. Co-authored-by: Codex <noreply@openai.com>

codex: align tests with lenient config enums

01c513f

Update test expectations after invalid enum values no longer fail whole config deserialization. Co-authored-by: Codex <noreply@openai.com>

jif-oai reviewed May 5, 2026

View reviewed changes

Comment thread codex-rs/config/src/config_toml.rs Outdated

Comment thread codex-rs/config/src/lenient.rs Outdated

Comment thread codex-rs/core/src/config/mod.rs

Comment thread codex-rs/config/src/config_toml.rs Outdated

Keep invalid config enums at load boundary

b1c09a4

chatgpt-codex-connector Bot reviewed May 6, 2026

View reviewed changes

Comment thread codex-rs/exec/src/lib.rs Outdated

aibrahim-oai added 3 commits May 6, 2026 15:42

Remove stale Lenient config references

883c70f

Fix lenient config CI findings

c1331e6

Document lenient config enum loading

e1d8a67

chatgpt-codex-connector Bot reviewed May 6, 2026

View reviewed changes

Comment thread codex-rs/app-server/src/config_manager_service_tests.rs Outdated

aibrahim-oai added 8 commits May 6, 2026 15:58

Keep app server invalid config write rejection

23f42dd

Remove nested invalid config enum leaves

e4ea70f

Use intermediate config enum loader

a8ce48a

Fix intermediate config enum warnings

7381525

Simplify intermediate enum warnings

0abc304

Keep lenient enum state explicit

f4a38f2

Inline lenient warning wrappers

22ca0e6

Order lenient config loader by visibility

2c8de33

chatgpt-codex-connector Bot reviewed May 6, 2026

View reviewed changes

aibrahim-oai added 3 commits May 7, 2026 02:23

Add config enum warning integration test

bd84525

Use spec loop for config enum warnings

bf4737b

Drive enum warnings from config schema

0e3631c

chatgpt-codex-connector Bot reviewed May 7, 2026

View reviewed changes

aibrahim-oai added 6 commits May 7, 2026 03:46

Keep enum warning scan nonblocking

4d88b99

Make enum config warnings advisory

c9c16fc

Document enum config warning walk

6c6dfa7

Simplify enum warning helpers

bf035fc

Add enum warning schema walk tests

28739bb

Fix config clippy lint

b3f8ccf

github-actions Bot mentioned this pull request May 7, 2026

📊 AI CLI 工具社区动态日报 2026-05-07 gsscsd/big_model_radar#305

Open

aibrahim-oai added 3 commits May 7, 2026 04:36

Fix enum config validation expectations

e6c51d5

Export enum config warning helper

6eb417e

Handle tagged enum config schema branches

b487b35

chatgpt-codex-connector Bot reviewed May 7, 2026

View reviewed changes

Comment thread codex-rs/config/src/state.rs

Keep invalid hook types advisory

f366ac3

chatgpt-codex-connector Bot reviewed May 7, 2026

View reviewed changes

Comment thread codex-rs/config/src/state.rs

github-actions Bot mentioned this pull request May 7, 2026

📊 AI CLI 工具社区动态日报 2026-05-07 ivanweng2077/big_model_radar#5

Open

Make nested enum config warnings non-blocking

a21345c

Align enum warning naming

dbb1528

chatgpt-codex-connector Bot reviewed May 7, 2026

View reviewed changes

Harden invalid enum warning paths

3f55561

chatgpt-codex-connector Bot reviewed May 7, 2026

View reviewed changes

aibrahim-oai added 2 commits May 7, 2026 06:40

Keep warned enum values non-fatal

fb66343

Fix config lock warning test expectation

d98019b

jif-oai reviewed May 7, 2026

View reviewed changes

jif-oai reviewed May 11, 2026

View reviewed changes

		#[serde_as(deserialize_as = "DefaultOnError")]
		pub approval_policy: Option<AskForApproval>,

		let (sanitized_layer, _typed_layer, layer_warnings) =
		deserialize_with_enum_warnings(normalized_layer)?;

	let mut config = Config::load_config_with_layer_stack(
	LOCAL_FS.as_ref(),
	lock_config_toml,
	harness_overrides,
	codex_home,
	lock_config_layer_stack,

		if let Some(warning) = enum_value_warnings(value).into_iter().next() {
		return Err(SerdeError::custom(warning));

Conversation

aibrahim-oai commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What Changed

Verification

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

jif-oai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 6, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 7, 2026

Choose a reason for hiding this comment

Uh oh!

aibrahim-oai commented May 7, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

aibrahim-oai commented May 7, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 7, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 7, 2026

Choose a reason for hiding this comment

Uh oh!

jif-oai left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

aibrahim-oai commented May 5, 2026 •

edited

Loading