Add remote thread config endpoint#18908
Conversation
rasmusrygaard
force-pushed
the
dev/rasmus/session_cfg_flag
branch
from
e3b5c3b to
fd9a3eb
Compare
rasmusrygaard
force-pushed
the
dev/rasmus/session_cfg_proto
branch
from
76adf66 to
77a8e29
Compare
rasmusrygaard
force-pushed
the
dev/rasmus/session_cfg_flag
branch
from
fd9a3eb to
c98d372
Compare
| cloud_requirements: Arc<RwLock<CloudRequirementsLoader>>, | ||
| arg0_paths: Arg0DispatchPaths, | ||
| thread_config_loader: Arc<dyn ThreadConfigLoader>, | ||
| thread_config_loader: Arc<RwLock<Arc<dyn ThreadConfigLoader>>>, |
There was a problem hiding this comment.
can we create loader early enough not to need hot-replace?
There was a problem hiding this comment.
The tricky part is that we depend on the config to resolve config settings, similar to how the cloud requirements depend on chatgpt_base_url. We can resolve the config values again here if we want to but that also seems a bit dirty
There was a problem hiding this comment.
Yeah, circular dependencies here are brutal 😢
rasmusrygaard
force-pushed
the
dev/rasmus/session_cfg_flag
branch
from
c98d372 to
8d5cbba
Compare
rasmusrygaard
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
codex/codex-rs/app-server-client/src/lib.rs
Line 390 in c98d372
Wire remote loader into in-process app-server startup
InProcessClientStartArgs::into_runtime_start_args always passes NoopThreadConfigLoader. Even when experimental_thread_config_endpoint is set in config, in-process app-server clients (used by TUI/exec paths) never instantiate RemoteThreadConfigLoader, so remote thread-scoped config is silently ignored in this runtime mode.
ℹ️ 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".
2 participants
Why
App-server needs a way to fetch thread-scoped config from the remote thread config service when the user config opts into that behavior. This mirrors the existing experimental remote thread store endpoint while keeping local/noop behavior as the default.
Startup paths also need to avoid silently dropping the remote config endpoint after the first config load. The stdio app-server path discovers the endpoint from the initial config and installs the real thread config loader for later config builds, while in-process clients used by TUI/exec now select the same remote loader directly from their provided config.
What changed
experimental_thread_config_endpointtoConfigToml,Config, andcore/config.schema.json.RemoteThreadConfigLoaderfrom the initially loaded config, falling back toNoopThreadConfigLoaderwhen unset.ConfigManagerreplace its thread config loader after startup discovery so later config loads use the selected loader.RemoteThreadConfigLoaderwhen its config hasexperimental_thread_config_endpointset.Verification
experimental_thread_config_endpoint_loads_from_config_toml.runtime_start_args_use_remote_thread_config_loader_when_configured.cargo check -p codex-app-server --lib.cargo test -p codex-app-server-client.