Skip to content

Conversation

@neiii
Copy link
Contributor

@neiii neiii commented Nov 21, 2025

Summary

Fixed goose configure command for Groq (d) provider. Previously, if the groq_api_key was unset, the cli would not ask for it and error out at the end of the setup. That said, I still think there are some issues with how goose handles Groq's conversations, with all of them being stuck on "Context limit reached. Compacting to continue conversation..."

Type of Change

  • Feature
  • Bug fix
  • Refactor / Code quality
  • Performance improvement
  • Documentation
  • Tests
  • Security fix
  • Build / Release
  • Other (specify below)

AI Assistance

  • This PR was created or reviewed with AI assistance
    Made with goose :P

Testing

Manual testing + the following:

cargo test -p goose --lib providers::factory::tests
cargo test -p goose --lib providers::formats::openai::tests
cargo test -p goose --lib providers::factory::tests::test_groq_provider_config_keys (per commit)

Screenshots/Demos (for UX changes)

Previously:
broken

Now
fixed

Copilot AI review requested due to automatic review settings November 21, 2025 04:38
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a bug in the goose configure command for declarative providers (like Groq) that use OpenAI-compatible engines. Previously, when configuring such providers, the CLI would display the base engine's API key environment variable (e.g., OPENAI_API_KEY) instead of the provider-specific one (e.g., GROQ_API_KEY), causing the configuration to fail when the provider-specific key was not set.

Key changes:

  • Modified register_with_name to update the first config key with the provider-specific api_key_env from the declarative configuration
  • Added test coverage to verify Groq provider uses GROQ_API_KEY instead of OPENAI_API_KEY

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
crates/goose/src/providers/provider_registry.rs Updates config_keys in declarative provider registration to use provider-specific API key environment variable
crates/goose/src/providers/factory.rs Adds test to verify Groq provider has correct config keys with GROQ_API_KEY

Copilot AI review requested due to automatic review settings November 21, 2025 05:04
neiii and others added 3 commits November 21, 2025 05:06
Signed-off-by: neiii <1mrtemeck1@gmail.com>
Signed-off-by: neiii <1mrtemeck1@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: neiii <1mrtemeck1@gmail.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Copy link
Collaborator

@michaelneale michaelneale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @neiii - just some nits (other comments) to fix, but also - can you explain more why it needs to set specifically the first key (ie overwrite whatever was there in the config), that seems odd as it would effect config for other providers too?

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@neiii
Copy link
Contributor Author

neiii commented Nov 21, 2025

thanks @neiii - just some nits (other comments) to fix, but also - can you explain more why it needs to set specifically the first key (ie overwrite whatever was there in the config), that seems odd as it would effect config for other providers too?

you're right, this is a rather silly implementation, not sure how I overlooked this. I'm currently debugging another implementation, by the looks of it, Groq is not the only affected provider; any openAI compatible provider is affected, including deepseek, and mistral. I'll update in a bit

Signed-off-by: neiii <1mrtemeck1@gmail.com>
@DOsinga
Copy link
Collaborator

DOsinga commented Nov 21, 2025

thanks @neiii - let's get groq work here completely. I agree with @michaelneale that blindly copying the key into the first parameter might have side effects though. a better but more involved solution would be to move the api_key out of the config_keys in ProviderMetaData and just have it there as an explicit field.

@neiii
Copy link
Contributor Author

neiii commented Nov 21, 2025

hey @DOsinga, I pushed a different implementation just as you were writing the message. Now, it gets the position of the OPENAI_API_KEY and then swaps it with the provider's API key. Looks good or would you like more change?

In terms of getting Groq fixed completely, quite frankly I'm not sure what's causing the issue, I've went back-and-forth with it for 2 hours yesterday, I'll tinker some more tonight. The requests go through (and it shows on Groq's dashboard), it's just that when Goose is processing them for some reason it sees it as a context overflow and tries to infinitely compress it, causing the requests to spam. I'll tinker some more tonight

@DOsinga
Copy link
Collaborator

DOsinga commented Nov 21, 2025

ok, thanks for the info, very useful. replacing OPEN_AI_KEY is a good start and helps for this for sure, but custom providers based on Anthropic (not very common to be sure) will run into the same issue. we can leave your solution in place though

as for groq, the reason this happens can be either that we have the token count on the models they return wrong so we think we're immediately out of context tokens, or there is something in the reply like "too many" and it is a 500 http error. If you can share a diagnostics report that could be helpful

@neiii
Copy link
Contributor Author

neiii commented Nov 23, 2025

@DOsinga I checked again with Groq now on a paid plan, the (bigger context) models seem to work, I suppose it was a tier issue (?). Is there anything else blocking for this change to be implemented?

Copy link
Collaborator

@DOsinga DOsinga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, looks good. simplify the tests though

Copy link
Collaborator

@michaelneale michaelneale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, mostly agree with @DOsinga on final nits, very cool!

Signed-off-by: neiii <1mrtemeck1@gmail.com>
Copilot AI review requested due to automatic review settings November 24, 2025 10:50
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Signed-off-by: neiii <1mrtemeck1@gmail.com>
@neiii
Copy link
Contributor Author

neiii commented Nov 24, 2025

@DOsinga compressed the tests and re-added the comment. Anything else?

Signed-off-by: neiii <1mrtemeck1@gmail.com>
Copilot AI review requested due to automatic review settings November 27, 2025 20:38
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@DOsinga DOsinga merged commit 0aca19e into block:main Dec 4, 2025
22 checks passed
tlongwell-block added a commit that referenced this pull request Dec 4, 2025
* origin/main:
  Update Anthropic and Google Gemini models to latest API versions (#5980)
  docs: chat recall tutorial (#5975)
  fix: `final assistant content cannot end with trailing whitespace` error from Anthropic (#5967)
  5527 multiple file popups (#5905)
  Groq configure fix  (#5833)
  added sidebar contextual information for firefox (#5433)
  docs: council of mine MCP (#5979)
  docs: nano banana extension (#5977)
  fix: remove prompt change, read model from config (#5976)
  Enable recipe deeplink parameters for pre-population (#5757)
  chore: upgrade npm packages (#5951)
  feat: ActionRequired (#5897)
  feat(acp): support loading sessions in acp (#5942)
  docs: add videos to multi-model page (#5938)
  docs: promote planning guide (#5934)
michaelneale added a commit that referenced this pull request Dec 5, 2025
* main:
  fix params not being substituted in activities (#5992)
  blog: MCP Sampling (#5987)
  Update Anthropic and Google Gemini models to latest API versions (#5980)
  docs: chat recall tutorial (#5975)
  fix: `final assistant content cannot end with trailing whitespace` error from Anthropic (#5967)
  5527 multiple file popups (#5905)
  Groq configure fix  (#5833)
  added sidebar contextual information for firefox (#5433)
  docs: council of mine MCP (#5979)
  docs: nano banana extension (#5977)
  fix: remove prompt change, read model from config (#5976)
  Enable recipe deeplink parameters for pre-population (#5757)
katzdave added a commit that referenced this pull request Dec 5, 2025
…nses-streaming

* 'main' of github.com:block/goose:
  blog: MCP Sampling (#5987)
  Update Anthropic and Google Gemini models to latest API versions (#5980)
  docs: chat recall tutorial (#5975)
  fix: `final assistant content cannot end with trailing whitespace` error from Anthropic (#5967)
  5527 multiple file popups (#5905)
  Groq configure fix  (#5833)
  added sidebar contextual information for firefox (#5433)
  docs: council of mine MCP (#5979)
  docs: nano banana extension (#5977)
  fix: remove prompt change, read model from config (#5976)
  Enable recipe deeplink parameters for pre-population (#5757)
  chore: upgrade npm packages (#5951)
  feat: ActionRequired (#5897)
  feat(acp): support loading sessions in acp (#5942)
  docs: add videos to multi-model page (#5938)
zanesq added a commit that referenced this pull request Dec 8, 2025
* 'main' of github.com:block/goose: (21 commits)
  Hide recipe icon in empty chat (#6022)
  docs: provider and model config (#6008)
  Show modal selector after configuring a provider (#6005)
  docs: additional mcp sampling resources (#6020)
  Flutter PR Code Review (#6011)
  feat(mcp): elicitation support (#5965)
  Onboarding detect provider from api key (#5955)
  Fix PATH on Windows for extensions (#6000)
  recipe(datahub): Add a recipe for searching & understanding data in DataHub (#5859)
  fix params not being substituted in activities (#5992)
  blog: MCP Sampling (#5987)
  Update Anthropic and Google Gemini models to latest API versions (#5980)
  docs: chat recall tutorial (#5975)
  fix: `final assistant content cannot end with trailing whitespace` error from Anthropic (#5967)
  5527 multiple file popups (#5905)
  Groq configure fix  (#5833)
  added sidebar contextual information for firefox (#5433)
  docs: council of mine MCP (#5979)
  docs: nano banana extension (#5977)
  fix: remove prompt change, read model from config (#5976)
  ...

# Conflicts:
#	ui/desktop/src/api/sdk.gen.ts
#	ui/desktop/src/components/bottom_menu/DirSwitcher.tsx
katzdave added a commit that referenced this pull request Dec 10, 2025
* 'main' of github.com:block/goose: (159 commits)
  Cleanup: Remove Recipe Key Flow (#6015)
  chore(deps): bump mdast-util-to-hast from 13.2.0 to 13.2.1 in /documentation (#5963)
  remove problematic corrupted woff font (#6006)
  Added search bar / filtering for recipes (#6019)
  Hide recipe icon in empty chat (#6022)
  docs: provider and model config (#6008)
  Show modal selector after configuring a provider (#6005)
  docs: additional mcp sampling resources (#6020)
  Flutter PR Code Review (#6011)
  feat(mcp): elicitation support (#5965)
  Onboarding detect provider from api key (#5955)
  Fix PATH on Windows for extensions (#6000)
  recipe(datahub): Add a recipe for searching & understanding data in DataHub (#5859)
  fix params not being substituted in activities (#5992)
  blog: MCP Sampling (#5987)
  Update Anthropic and Google Gemini models to latest API versions (#5980)
  docs: chat recall tutorial (#5975)
  fix: `final assistant content cannot end with trailing whitespace` error from Anthropic (#5967)
  5527 multiple file popups (#5905)
  Groq configure fix  (#5833)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants