Add remote thread config loader protos#18892
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96609c22b3
ℹ️ 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".
wiltzius-openai
left a comment
There was a problem hiding this comment.
seems reasonable
I think there's no local implementation yet? I opted to try to keep the local implementation of the interface ahead of the remote implementation, mainly because as I plumbed in the local version I found various details of the interface needed to change. Not a requirement just a thought about ordering.
| @@ -0,0 +1,19 @@ | |||
| use std::path::PathBuf; | |||
|
|
|||
| fn main() -> Result<(), Box<dyn std::error::Error>> { | |||
There was a problem hiding this comment.
would love a better place to put the proto generation than examples but I couldn't figure out a better way to do it with the repo build rules
|
Good flag! Local implementation is a no-op because since we don't inject any new config. I have the wiring in a stacked branch on top of this, but it just injects the NoOp loader |
76adf66 to
77a8e29
Compare
Why
Thread-scoped config needs a stable boundary between the app/session owner and the config stack. Instead of having call sites manually copy thread config fields into individual overrides, this adds the proto and Rust plumbing needed for a
ThreadConfigLoaderimplementation to return typed sources that can be translated into ordinary config layer entries.Keeping the remote payload typed also makes precedence easier to reason about: session-owned thread config maps back to the existing session config source, while user-owned thread config is represented separately without introducing a new config-layer source until it has TOML-backed fields.
What changed
codex.thread_config.v1protobuf service and generated Rust module for loading thread config sources.RemoteThreadConfigLoader, which calls the gRPC service, parsesSessionThreadConfig/UserThreadConfig, and validates provider fields such aswire_api, auth timeout, and absolute auth cwd.config/scripts/generate-proto.shandconfig/examples/generate-proto.rs.ThreadConfigLoader::load_config_layers, plus static/no-op loader helpers, so tests and callers can use the same typed loader interface while config-layer translation stays centralized.Verification
cargo test -p codex-config thread_config