Preserve session MCP config on refresh#21055
Conversation
aaronl-openai
changed the title
…verlay-on-mcp-refresh # Conflicts: # codex-rs/app-server/src/codex_message_processor.rs # codex-rs/app-server/src/message_processor.rs
…verlay-on-mcp-refresh
dylan-hurd-oai
left a comment
dylan-hurd-oai
left a comment
There was a problem hiding this comment.
I am onboard with the general principle here unless @xl-openai has issues, but I think i'd propose restructuring this a bit:
- I'm not convinced we should add the purpose-specific refresh logic to codex-core config, it seems like this might be better served using a generic refresh interface (ideally an existing one) with app_server::config_manager?
- It seems like our current approach seems just thread more state through spawn_effective_plugins_changed_task - can we do this in a simpler way?
| } | ||
| } | ||
|
|
||
| pub async fn rebuild_for_mcp_refresh_planning( |
There was a problem hiding this comment.
I'm curious why we need a purpose-specific rebuild function here?
There was a problem hiding this comment.
I renamed to rebuild_preserving_session_layers to make this more clearly generic. it does feel a bit weird that combining thread-level config on top of the refreshed config is so manual, but I think it's necessary.
There was a problem hiding this comment.
ack, yeah i think we should rethink this generally
Yeah, this one I'm not too sure about. The main reason
we're passing in the config manager instead of a config object here because refresh needs to take each thread's config into account, rather than applying the same global config to all threads. I'm not sure this is avoidable since we really do need more state to know how each thread is configured. |
…verlay-on-mcp-refresh # Conflicts: # codex-rs/app-server/src/request_processors/mcp_processor.rs
2 participants
Overview
MCP refreshes were rebuilding active threads from fresh disk-backed config only, which dropped thread-start session overlays such as app-injected MCP servers. This keeps refreshes current with disk config while preserving the thread-local config that only the active thread knows about.
Changes
cwd, rather than fanning out one app-server config to every thread.SessionFlagslayer while replacing reloadable config layers with freshly loaded config, then derive the MCP refresh payload from the rebuilt result.Verification
cargo test -p codex-app-server --lib.