[app-server] fix config loading for conversations#8765
Conversation
owenlin0
left a comment
owenlin0
left a comment
There was a problem hiding this comment.
approving to get --config flags working for app-server, but +1 to unifying config code / making it easier to follow
owenlin0
left a comment
owenlin0
left a comment
There was a problem hiding this comment.
btw the fix ultimately was that we never read &self.cli_overrides from codex_message_processor? nice find!
yup yup! I agree it's confusing because cli_overrides is an overloaded term and easy to miss when we are not passing in all the overrides. will rename first to make it clearer |
bolinfest
left a comment
bolinfest
left a comment
There was a problem hiding this comment.
I think some terminology needs to be tightened up here. The things that are currently called harness_overrides are not defined by the harness but a NewConversationParams, so I think some other name is in order.
celia-oai
force-pushed
the
dev/cc/fix-config
branch
2 times, most recently
from
726dc32 to
fa76165
Compare
celia-oai
force-pushed
the
dev/cc/fix-config
branch
from
fa76165 to
020784d
Compare
|
Lots of changes on this recently so I rely on @.bolinfest for this |
celia-oai
force-pushed
the
dev/cc/fix-config
branch
from
020784d to
15774aa
Compare
celia-oai
force-pushed
the
dev/cc/fix-config
branch
from
15774aa to
5027fd7
Compare
celia-oai
force-pushed
the
dev/cc/fix-config
branch
2 times, most recently
from
599d690 to
1d69d0d
Compare
celia-oai
force-pushed
the
dev/cc/fix-config
branch
from
1d69d0d to
3a4a023
Compare
bolinfest
left a comment
bolinfest
left a comment
There was a problem hiding this comment.
ok, let's go with the typesafe_overrides thing and this is good by me: thanks for fixing!
Co-authored-by: Michael Bolin <mbolin@openai.com>
celia-oai
force-pushed
the
dev/cc/fix-config
branch
from
a286130 to
ad4ba98
Compare
4 participants
Currently we don't load config properly for app server conversations. see: https://linear.app/openai/issue/CODEX-3956/config-flags-not-respected-in-codex-app-server. This PR fixes that by respecting the config passed in.
Tested by running
cargo build -p codex-cli && RUST_LOG=codex_app_server=debug CODEX_BIN=target/debug/codex cargo run -p codex-app-server-test-client -- \ --config model_providers.mock_provider.base_url=\"http://localhost:4010/v2\" \ --config model_provider=\"mock_provider\" \ --config model_providers.mock_provider.name="hello" \ send-message-v2 "hello"and verified that the mock_provider is called instead of default provider.
#closes https://linear.app/openai/issue/CODEX-3956/config-flags-not-respected-in-codex-app-server