[codex] Fix runtime config loading and invalid model fallback#301
Merged
[codex] Fix runtime config loading and invalid model fallback#301
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes two runtime issues that were showing up together in the Web UI.
The first issue was in Runtime Config. The Settings dialog only opened after
/api/configand/api/config/self_checkboth returned and after the frontend had finished building the full config draft. In practice that made the button feel unresponsive, especially on slower instances. The UI also did not expose Weixin in the Web settings even though the backend already supported it.The second issue was a runtime failure path around invalid model ids, notably
*. Users could end up with*in effective model selection through stale config, profile data, or in-memory overrides. When that happened the request was forwarded to the provider and failed with errors like* is not a valid model ID. Separately,/api/configcould return 500 because the config response path serializedConfigdirectly through JSON even though it containsserde_yaml::Valuetrees, which can include mapping keys that are not plain JSON strings.Root cause:
The frontend treated opening Runtime Config as a blocking fetch-and-hydrate workflow instead of opening the shell immediately and loading inside it. On the backend, model selection trusted incoming config and override values too much, and the config API assumed direct JSON serialization of YAML-backed config structures was always safe.
Fixes in this PR:
*are ignored and fall back to the provider default model./model *explicitly in chat commands with a user-facing validation message./api/configserialize through a safe YAML-to-JSON conversion path before redacting secrets so non-string YAML mapping keys no longer crash the endpoint.Validation performed:
npm --prefix web run buildcargo test wildcard -- --nocapturecargo test invalid_runtime_model_override -- --nocapturecargo test test_redact_config -- --nocapture