Skip to content

Make missing config clears no-ops#20334

Merged
jif-oai merged 1 commit into
mainfrom
etraut/config-clear-missing-noop
Apr 30, 2026
Merged

Make missing config clears no-ops#20334
jif-oai merged 1 commit into
mainfrom
etraut/config-clear-missing-noop

Conversation

@etraut-openai

Copy link
Copy Markdown
Collaborator

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

@jif-oai jif-oai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make sense

@jif-oai jif-oai merged commit a73403a into main Apr 30, 2026
25 checks passed
@jif-oai jif-oai deleted the etraut/config-clear-missing-noop branch April 30, 2026 08:13
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 30, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

app-server config/value/write returns configPathNotFound when clearing an unset nested config value

2 participants