feat: allow GOOSE_CLI_SHOW_THINKING to be set in config.yaml#8097
feat: allow GOOSE_CLI_SHOW_THINKING to be set in config.yaml#8097DOsinga merged 3 commits intoblock:mainfrom
Conversation
Previously this could only be set as an environment variable. Now it can also be set in config.yaml as a boolean, matching how other goose settings work. Signed-off-by: Dale Lakes <[email protected]> Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
r0x0d
left a comment
r0x0d
left a comment
There was a problem hiding this comment.
It looks good to me. Should we have some type of test to verify that this will work correctly?
| let from_config = Config::global() | ||
| .get_param::<bool>("GOOSE_CLI_SHOW_THINKING") | ||
| .unwrap_or(false); | ||
| (from_env || from_config) && std::io::stdout().is_terminal() |
There was a problem hiding this comment.
Config already reads from the env variables, so this is double. I'll fix.
Config::get_param already checks env vars (uppercased) before falling back to config.yaml, so the separate env-var check and static LazyLock are unnecessary.
DOsinga
left a comment
DOsinga
left a comment
There was a problem hiding this comment.
Config::get_param already checks the env var (uppercased key) before falling back to config.yaml, so the separate env-var check and the LazyLock static were redundant. Simplified to a direct call in should_show_thinking().
![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: 419ff898de
ℹ️ 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".
| .get_param::<bool>("GOOSE_CLI_SHOW_THINKING") | ||
| .unwrap_or(false) |
There was a problem hiding this comment.
Switching to get_param::<bool>("GOOSE_CLI_SHOW_THINKING") changes the environment-variable contract from “set means enabled” to “must parse as boolean”. In this path, values like GOOSE_CLI_SHOW_THINKING=1 deserialize as a number and then fail bool deserialization, so unwrap_or(false) silently disables thinking output for existing users who followed current docs/examples. This is a behavior regression introduced by this change.
Useful? React with 👍 / 👎.
| Config::global() | ||
| .get_param::<bool>("GOOSE_CLI_SHOW_THINKING") | ||
| .unwrap_or(false) |
There was a problem hiding this comment.
should_show_thinking() is called from streaming rendering for each MessageContent::Thinking chunk, and it now calls Config::global().get_param(...) every time. get_param falls back to load()/load_raw() when the env var is unset, which performs config file reads/migration checks on each invocation; this adds avoidable per-chunk I/O in the hot path and can degrade streaming responsiveness compared with the previous LazyLock behavior.
Useful? React with 👍 / 👎.
* main: (337 commits) fix: replace panics with user-friendly errors in CLI session builder (#7901) fix: read GOOSE_CONTEXT_LIMIT from config.yaml, not just env vars (#7900) fix: deliver truncation notice as separate content block (#7899) fix: use platform-appropriate commands in developer extension instructions (#7898) fix: replace any with proper SVG types in icon components (#7873) chore: remove debug console.log statements, stale comments, and dead code (#8142) feat: Gemini OAuth provider (#8129) chore(deps): bump picomatch from 2.3.1 to 2.3.2 in /documentation (#8123) feat: show installed skills in UI (#7910) fix(deps): gate keyring platform features behind target-specific deps (#8039) chore(deps): bump yaml from 2.8.2 to 2.8.3 in /evals/open-model-gym/suite (#8124) fix: strip message wrapper in CLI session title generation (#7996) fix(providers): fall back to configured models when models endpoint fetch fails (#7530) chore(deps): bump brace-expansion from 5.0.3 to 5.0.5 in /evals/open-model-gym/suite (#8139) fix: prevent Ollama provider from hanging on tool-calling requests (#7723) fix: VMware Tanzu Platform provider - bug fixes, streaming, UI improvements (#8126) feat: allow GOOSE_CLI_SHOW_THINKING to be set in config.yaml (#8097) fix: GitHub Copilot auth fails to open browser in Desktop app (#6957) (#8019) fix(ci): produce .tar.gz archives for Zed ACP registry compatibility (#8054) feat: add GOOSE_SHOW_FULL_OUTPUT config to disable tool output truncation (#7919) ... # Conflicts: # crates/goose/src/providers/formats/openai.rs
) Signed-off-by: Dale Lakes <[email protected]> Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]> Co-authored-by: Douwe Osinga <[email protected]> Signed-off-by: Cameron Yick <[email protected]>
* main: (337 commits) fix: replace panics with user-friendly errors in CLI session builder (#7901) fix: read GOOSE_CONTEXT_LIMIT from config.yaml, not just env vars (#7900) fix: deliver truncation notice as separate content block (#7899) fix: use platform-appropriate commands in developer extension instructions (#7898) fix: replace any with proper SVG types in icon components (#7873) chore: remove debug console.log statements, stale comments, and dead code (#8142) feat: Gemini OAuth provider (#8129) chore(deps): bump picomatch from 2.3.1 to 2.3.2 in /documentation (#8123) feat: show installed skills in UI (#7910) fix(deps): gate keyring platform features behind target-specific deps (#8039) chore(deps): bump yaml from 2.8.2 to 2.8.3 in /evals/open-model-gym/suite (#8124) fix: strip message wrapper in CLI session title generation (#7996) fix(providers): fall back to configured models when models endpoint fetch fails (#7530) chore(deps): bump brace-expansion from 5.0.3 to 5.0.5 in /evals/open-model-gym/suite (#8139) fix: prevent Ollama provider from hanging on tool-calling requests (#7723) fix: VMware Tanzu Platform provider - bug fixes, streaming, UI improvements (#8126) feat: allow GOOSE_CLI_SHOW_THINKING to be set in config.yaml (#8097) fix: GitHub Copilot auth fails to open browser in Desktop app (#6957) (#8019) fix(ci): produce .tar.gz archives for Zed ACP registry compatibility (#8054) feat: add GOOSE_SHOW_FULL_OUTPUT config to disable tool output truncation (#7919) ... # Conflicts: # crates/goose/src/providers/formats/openai.rs
3 participants
Previously
GOOSE_CLI_SHOW_THINKINGcould only be set as an environment variable. Now it can also be set in config.yaml as a boolean, matching how other goose settings work.