Using cached connector directory for discoverable tools list#21497
Open
mzeng-openai wants to merge 8 commits intomainfrom
Open
Using cached connector directory for discoverable tools list#21497mzeng-openai wants to merge 8 commits intomainfrom
mzeng-openai wants to merge 8 commits intomainfrom
Conversation
597b264 to
86745f4
Compare
xl-openai
approved these changes
May 7, 2026
xl-openai
approved these changes
May 7, 2026
mzeng-openai
commented
May 8, 2026
|
|
||
| pub fn cached_all_connectors(cache_key: &AllConnectorsCacheKey) -> Option<Vec<AppInfo>> { | ||
| let mut cache_guard = ALL_CONNECTORS_CACHE | ||
| pub fn cached_directory_connectors( |
Collaborator
Author
There was a problem hiding this comment.
renamed for better semantics
mzeng-openai
commented
May 8, 2026
| return Some(cached_connectors); | ||
| } | ||
|
|
||
| let directory_cache::CachedConnectorDirectoryDiskLoad::Hit { connectors } = |
Collaborator
Author
There was a problem hiding this comment.
Disk cache hydrates in-memory cache at first read.
mzeng-openai
commented
May 8, 2026
| pub fn cached_directory_connectors( | ||
| cache_context: &ConnectorDirectoryCacheContext, | ||
| ) -> Option<Vec<AppInfo>> { | ||
| if let Some(cached_connectors) = cached_directory_connectors_in_memory(&cache_context.cache_key) |
Collaborator
Author
There was a problem hiding this comment.
in-memory directory cache is still the source-of-truth
mzeng-openai
commented
May 8, 2026
| list_directory_connectors_for_tool_suggest_with_auth(config, auth).await?; | ||
| let connector_ids = tool_suggest_connector_ids(config).await; | ||
| let directory_connectors = codex_connectors::merge::merge_plugin_connectors( | ||
| cached_directory_connectors_for_tool_suggest_with_auth(config, auth).await, |
Collaborator
Author
There was a problem hiding this comment.
Always constructs directory data from cache because it affects the startup path (prewarm).
For the first run in a new environment, it will not block startup but can cause a kv cache miss if the directory cache is empty at the time the prompt is sent and cache is refreshed later. This will not be an issue when plugin suggestion is migrated to search-based approach.
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.
Summary
Startup tool construction currently depends on connector directory metadata for
tool_suggestdiscoverables. On a cold directory cache, that can put slow connector-directory requests on the blocking path even though the tools array only needs directory data for install suggestions, not for the live connector MCP tools themselves.This PR keeps the discoverables path off that cold network fetch:
~/.codex/cache/codex_app_directory/<hash>.jsonand use it to hydrate the in-memory cache on later runs before the normal refresh path updates itThis reduces first-turn startup work without changing how live connector MCP tools are sourced. Longer term, directory-backed install suggestions should move to a search-based flow so they no longer need to be inlined into the tools prompt at all.
Testing
cargo test -p codex-connectorscargo test -p codex-chatgptcargo test -p codex-core request_plugin_install_is_available_without_search_tool_after_discovery_attemptscargo test -p codex-core tool_suggest_uses_connector_id_fallback_when_directory_cache_is_empty