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>
![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
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.
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 👍 / 👎.
The config value was declared as String, which caused serde_yaml to fail when deserializing a bare numeric YAML value (e.g. 600). The error was silently swallowed by .ok(), making the global config override ineffective. Switch to the two-arg config_value! form with u64 so both YAML integers and env var strings (parsed via parse_env_value) deserialize correctly. Drop the now-unnecessary .parse::<u64>() chain in resolve_timeout. Signed-off-by: Douwe Osinga <douwe@squareup.com>
DOsinga
left a comment
DOsinga
left a comment
There was a problem hiding this comment.
Looks good. Pushed a fix for the codex comment: changed GOOSE_DEFAULT_EXTENSION_TIMEOUT from String to u64 config type. The String variant caused serde_yaml to fail on bare numeric YAML values (e.g. 600), and the error was silently swallowed by .ok(), making the global config override ineffective. Also merged main.
* origin/main: (32 commits) docs: rework homepage and add aaif migration blog post (#8356) chore(aaif): rename a bunch of repository references (#8152) fix: use OPENAI_API_KEY secret for recipe security scanner (#8358) feat: configurable extension timeouts via ACP _meta and global default (#8295) fix: hide hidden extensions in UI (#8346) refactor: skills as its own platform ext (#8244) fix baseUrl (#8347) Fix desktop slash commands (#8341) fix(cli): display platform-correct secrets path in keyring config dialog (#8328) feat(acp): add reusable ACP provider controls (#8314) fix: resolve MDX compilation error in using-goosehints.md (#8332) fix: use v1beta1 API version for Google/MaaS models on GCP Vertex AI (#8278) docs: add MCP Roots guide (#8252) rust acp client for extension methods (#8227) fix: reconsolidate split tool-call messages to follow OpenAI format (#7921) fix: clean up MCP subprocesses after abrupt parent exit (#8242) build: raise default stack reserve to 8 MB (#8234) fix(config): honour GOOSE_DISABLE_KEYRING from config.yaml at startup (#8219) feat: add configurable fast_model for declarative providers (#8194) fix(authentication): Allow connecting to Oauth servers that use protected-resource fallback instead of the WWW-authenticate header (#8148) ...
2 participants
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