From a73403a89097ad9a5e25c04c8b75f839d6a2fabd Mon Sep 17 00:00:00 2001
From: Eric Traut <etraut@openai.com>
Date: Thu, 30 Apr 2026 01:13:33 -0700
Subject: [PATCH] Make missing config clears no-ops (#20334)

## Why

Fixes #20145.

`config/value/write` treats a JSON `null` value as a request to clear
the config key. Clearing a key that is already absent should be
idempotent, but clearing a nested key such as `features.personality`
from an empty `config.toml` returned `configPathNotFound` because
`clear_path` treated the missing `features` parent table as an error.

That makes app-server reset flows brittle because clients have to read
first and avoid sending a clear request unless the parent path already
exists.

## What Changed

- Updated app-server config clearing so missing intermediate tables, or
non-table parents, are treated as an unchanged no-op.
- Removed the now-unreachable `MergeError::PathNotFound` path from
config write merging.
- Added a regression test covering `features.personality = null` against
an empty user config.

## Verification

- `cargo test -p codex-app-server clear_missing_nested_config_is_noop`
- `cargo test -p codex-app-server` was run; the config manager unit
suite passed, but one unrelated integration test failed because
`turn_start_emits_thread_scoped_warning_notification_for_trimmed_skills`
expected `7` trimmed skills and observed `8`.
- `just fix -p codex-app-server`
---
 .../app-server/src/config_manager_service.rs  | 14 +++++------
 .../src/config_manager_service_tests.rs       | 24 +++++++++++++++++++
 2 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/codex-rs/app-server/src/config_manager_service.rs b/codex-rs/app-server/src/config_manager_service.rs
index ec4a1a680..aef4393a0 100644
--- a/codex-rs/app-server/src/config_manager_service.rs
+++ b/codex-rs/app-server/src/config_manager_service.rs
@@ -244,10 +244,6 @@ impl ConfigManager {
 
             apply_merge(&mut user_config, &segments, parsed_value.as_ref(), strategy).map_err(
                 |err| match err {
-                    MergeError::PathNotFound => ConfigManagerError::write(
-                        ConfigWriteErrorCode::ConfigPathNotFound,
-                        "Path not found",
-                    ),
                     MergeError::Validation(message) => ConfigManagerError::write(
                         ConfigWriteErrorCode::ConfigValidationError,
                         message,
@@ -413,7 +409,6 @@ fn parse_key_path(path: &str) -> Result<Vec<String>, String> {
 
 #[derive(Debug)]
 enum MergeError {
-    PathNotFound,
     Validation(String),
 }
 
@@ -485,14 +480,17 @@ fn clear_path(root: &mut TomlValue, segments: &[String]) -> Result<bool, MergeEr
     for segment in parents {
         match current {
             TomlValue::Table(table) => {
-                current = table.get_mut(segment).ok_or(MergeError::PathNotFound)?;
+                let Some(next) = table.get_mut(segment) else {
+                    return Ok(false);
+                };
+                current = next;
             }
-            _ => return Err(MergeError::PathNotFound),
+            _ => return Ok(false),
         }
     }
 
     let Some(parent) = current.as_table_mut() else {
-        return Err(MergeError::PathNotFound);
+        return Ok(false);
     };
 
     Ok(parent.remove(last).is_some())
diff --git a/codex-rs/app-server/src/config_manager_service_tests.rs b/codex-rs/app-server/src/config_manager_service_tests.rs
index 108254859..e9b0b3c76 100644
--- a/codex-rs/app-server/src/config_manager_service_tests.rs
+++ b/codex-rs/app-server/src/config_manager_service_tests.rs
@@ -106,6 +106,30 @@ personality = true
     Ok(())
 }
 
+#[tokio::test]
+async fn clear_missing_nested_config_is_noop() -> Result<()> {
+    let tmp = tempdir().expect("tempdir");
+    let path = tmp.path().join(CONFIG_TOML_FILE);
+    std::fs::write(&path, "")?;
+
+    let service = ConfigManager::without_managed_config_for_tests(tmp.path().to_path_buf());
+    let response = service
+        .write_value(ConfigValueWriteParams {
+            file_path: Some(path.display().to_string()),
+            key_path: "features.personality".to_string(),
+            value: serde_json::Value::Null,
+            merge_strategy: MergeStrategy::Replace,
+            expected_version: None,
+        })
+        .await
+        .expect("clear missing config succeeds");
+
+    assert_eq!(response.status, WriteStatus::Ok);
+    assert_eq!(response.overridden_metadata, None);
+    assert_eq!(std::fs::read_to_string(&path)?, "");
+    Ok(())
+}
+
 #[tokio::test]
 async fn write_value_supports_nested_app_paths() -> Result<()> {
     let tmp = tempdir().expect("tempdir");