feat: configurable extension timeouts via ACP _meta and global default#8295
feat: configurable extension timeouts via ACP _meta and global default#8295
Conversation
Add two ways to control extension timeouts: 1. Per-extension: read an optional 'timeout' value from the ACP _meta field on McpServer configurations. Both Stdio and Http variants of mcp_server_to_extension_config pass this through to ExtensionConfig. 2. Global default: add GOOSE_DEFAULT_EXTENSION_TIMEOUT config value (settable in config.yaml or as an environment variable). All three extension client creation paths (child_process_client, create_streamable_http_client, and builtin extensions) now use a resolve_timeout() helper that checks: 1. Per-extension timeout (from config or ACP _meta) - highest priority 2. GOOSE_DEFAULT_EXTENSION_TIMEOUT global config value 3. Hardcoded DEFAULT_EXTENSION_TIMEOUT constant (300s) - fallback Signed-off-by: Bradley Axen <baxen@squareup.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fdbde22a58
ℹ️ 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".
| config_value!(CLAUDE_THINKING_TYPE, String); | ||
| config_value!(CLAUDE_THINKING_EFFORT, String); | ||
| config_value!(CLAUDE_THINKING_BUDGET, i32); | ||
| config_value!(GOOSE_DEFAULT_EXTENSION_TIMEOUT, String, "300"); |
There was a problem hiding this comment.
Parse default extension timeout as numeric config
GOOSE_DEFAULT_EXTENSION_TIMEOUT is registered as String (config_value!(..., String, "300")), but users will naturally set this as a numeric YAML/env value (e.g. 600). In Config::get_param, numeric env/YAML values are deserialized as numbers, which cannot deserialize into the generated string wrapper, so get_goose_default_extension_timeout() errors and resolve_timeout() silently falls back to DEFAULT_EXTENSION_TIMEOUT (300). This makes the new global timeout override ineffective for common configurations unless the value is quoted as a string.
Useful? React with 👍 / 👎.
Adds support for configuring per-extension timeouts through two mechanisms:
_meta.timeout: Extensions can specify a timeout (in seconds) via the_metafield on Stdio and HTTP MCP server configs, which is extracted and passed through toExtensionConfig.GOOSE_DEFAULT_EXTENSION_TIMEOUTconfig value (defaulting to 300s) allows overriding the hardcoded default timeout for all extensions.The timeout resolution order is: per-extension
_meta.timeout→GOOSE_DEFAULT_EXTENSION_TIMEOUTconfig → hardcoded 300s default.Changes
goose-acp/src/server.rs: Extract timeout from_metawhen convertingMcpServertoExtensionConfiggoose/src/agents/extension_manager.rs: Centralize timeout resolution inresolve_timeout()helper, checking config before falling back to the constantgoose/src/config/base.rs: AddGOOSE_DEFAULT_EXTENSION_TIMEOUTconfig value