Using cached connector directory for discoverable tools list#21497
Conversation
mzeng-openai
changed the title
mzeng-openai
force-pushed
the
dev/mzeng/cached-tool-suggest-discoverables
branch
from
597b264 to
86745f4
Compare
|
|
||
| pub fn cached_all_connectors(cache_key: &AllConnectorsCacheKey) -> Option<Vec<AppInfo>> { | ||
| let mut cache_guard = ALL_CONNECTORS_CACHE | ||
| pub fn cached_directory_connectors( |
There was a problem hiding this comment.
renamed for better semantics
| return Some(cached_connectors); | ||
| } | ||
|
|
||
| let directory_cache::CachedConnectorDirectoryDiskLoad::Hit { connectors } = |
There was a problem hiding this comment.
Disk cache hydrates in-memory cache at first read.
| 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) |
There was a problem hiding this comment.
in-memory directory cache is still the source-of-truth
| 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, |
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.
pakrym-oai
left a comment
pakrym-oai
left a comment
There was a problem hiding this comment.
Worth adding an integration test?
There is already an integration test for the main user-visible regression in requeset_plugin_install.rs! |
3 participants
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