Skip to content

Conversation

@zanesq
Copy link
Collaborator

@zanesq zanesq commented Dec 2, 2025

Summary

Resurrecting #5477 from @spencrmartin and integrating the api key detection logic from @DOsinga in #5147

verified working locally

image

spencrmartin and others added 25 commits October 30, 2025 09:55
- Add auto_detect.rs module for parallel provider testing
- Add /config/detect-provider API endpoint
- Add DetectProviderRequest and DetectProviderResponse structs
- Update OpenAPI specifications
- Backend compiles successfully

Part of PR #4950 + #5147 integration
- Add ApiKeyTester component calling /config/detect-provider
- Update ProviderGuard with ApiKeyTester in onboarding
- Add provider icons and regenerate API client
- Show progress and results during detection

Integration of PR #4950 + #5147
- Make API Key Tester the only grey container (bg-background-muted)
- Arrange Tetrate and OpenRouter side-by-side in grid layout
- Add 'Other Providers' section with link to settings
- Use transparent backgrounds for provider cards
- Improve visual hierarchy and spacing
- Add provider icons to each card
- Maintain hover effects and transitions

Layout now follows:
[Quick Setup with API Key] (grey)
[Tetrate] [OpenRouter] (transparent, side-by-side)
[Other Providers] (transparent)
- Change /settings/providers to /configure-providers
- Fixes 'No routes matched location' error in console
- Allows 'Other Providers' button to work properly
- Add DESIGN_SUMMARY.md with layout improvements overview
- Add INTEGRATION_COMPLETE.md with full integration details
- Add ROUTE_FIX_INSTRUCTIONS.md for troubleshooting
- Update API client files from regeneration
- Remove integration documentation files (kept locally for reference)
- Remove backup files
- Keep only production code changes
- Replace 'error: any' with 'error: unknown'
- Add proper ApiError interface for type safety
- Improve error handling with proper type guards
- Fixes ESLint @typescript-eslint/no-explicit-any warning
- Reorder imports alphabetically in config_management.rs
- Move auto_detect import to correct position in mod.rs
- Remove extra blank lines
- Fixes cargo fmt --check validation
Backend changes:
- Detect provider from key format first (sk-ant- = Anthropic, sk- = OpenAI, etc.)
- Only test the detected provider instead of all providers
- Return structured error responses with suggestions
- Add DetectProviderError struct with detailed feedback

Frontend changes:
- Handle new structured error responses
- Show provider-specific error messages
- Display helpful suggestions when validation fails
- Improve loading state to show 'Testing API key...'

Fixes issue where wrong keys would test against all providers.
Now users get clear feedback about their specific key type.
- Add userInActiveSetup state to track when user is actively testing keys
- Prevent ProviderGuard from redirecting when user is in setup flow
- Add onStartTesting callback to ApiKeyTester component
- Fix useEffect dependencies to include userInActiveSetup
- Add frontend rejection of Ollama fallbacks in Quick Setup
- Add specific handling for OpenRouter keys (sk-or- format)
- Remove suggestions box and test results header for cleaner UI
- Add debug logging for troubleshooting

Fixes issue where users entering incorrect API keys would see error
but then get redirected to existing provider configurations instead
of staying on setup screen to retry.
- Add disable_ollama_fallback parameter to DetectProviderRequest
- Update auto_detect function to respect fallback disable flag
- Update OpenAPI schema and TypeScript types
- Prepare for future backend compilation

Note: These backend changes require Rust recompilation to take effect.
Frontend workarounds are in place until backend can be rebuilt.
- Replace format-based detection with parallel testing approach
- Test all providers simultaneously using tokio::spawn for better accuracy
- Simplify API response (404 for no match instead of structured errors)
- Remove complex Ollama rejection logic - let parallel testing handle it
- Should properly detect Anthropic, OpenAI, Google, Groq, xAI, and Ollama
- Maintains existing Quick Setup UI with improved backend detection

This integrates the robust parallel testing approach from PR #5147
into our Quick Setup onboarding flow for better API key detection.
- Add detect_cloud_provider_from_api_key() function that excludes Ollama
- Add /config/detect-cloud-provider endpoint for Quick Setup use
- Update ApiKeyTester to use cloud-only detection endpoint
- Remove frontend Ollama rejection since backend won't return it
- Ensures Quick Setup only works with cloud API providers
- Maintains full detection (including Ollama) for other use cases

This prevents unwanted Ollama fallbacks in Quick Setup while preserving
the parallel testing approach from PR #5147 for better accuracy.
The new /config/detect-cloud-provider endpoint requires backend recompilation.
Until then, use the existing /config/detect-provider endpoint with frontend
filtering to reject Ollama results in Quick Setup.

- Use detectProvider() instead of detectCloudProvider()
- Add frontend Ollama rejection back
- Maintains cloud-only behavior for Quick Setup
- Works with current compiled backend
- Change from module import to specific function imports
- Fix function calls to not use module prefix
- Resolves compilation error for detect_cloud_provider_from_api_key

This allows the backend to compile with the new cloud-only detection
endpoint when the Rust code is recompiled.
The Ollama rejection logic was accidentally removed in previous changes.
This adds it back to prevent unwanted Ollama fallbacks in Quick Setup.

- Reject any Ollama detection results in Quick Setup flow
- Show proper error message for cloud providers only
- Return early to prevent success flow execution
- Maintains focus on cloud API providers for Quick Setup
The pure parallel approach from PR #5147 was testing Anthropic keys against
OpenAI API, causing incorrect rejections. This hybrid approach:

1. Detects key format first (sk-ant- → anthropic)
2. Tests the likely provider first with the key
3. Falls back to parallel testing if format detection fails
4. Includes OpenRouter support (sk-or- format)

This should correctly route sk-ant- keys to Anthropic API for proper validation
while maintaining the robustness of parallel testing for unknown formats.

Benefits:
- Anthropic keys test against Anthropic API (not OpenAI)
- Faster detection (test likely provider first)
- Robust fallback (parallel testing if needed)
- Supports more providers (OpenRouter)
Since backend hybrid detection requires recompilation, add frontend
format detection to provide better error messages and debugging.

- Detect key format on frontend (sk-ant- → Anthropic, etc.)
- Provide format-specific error messages when validation fails
- Add debugging to show detected format and key analysis
- Better user guidance based on detected provider format

This helps users understand why their keys are failing validation
even when the backend is still using the old parallel-only approach.
- Enhanced frontend format detection for all major providers (Anthropic, OpenRouter, OpenAI, Google, Groq, xAI)
- Added smart workarounds for backend parallel testing issues
- Prevent unwanted redirects during API key testing by maintaining userInActiveSetup state
- Reject Ollama fallback in Quick Setup to ensure cloud-only provider detection
- Configure providers directly when backend detection fails but format is recognized
- Use correct model names: claude-3-haiku-20240307 for Anthropic, anthropic/claude-3-haiku for OpenRouter
- Add input validation to prevent console log injection issues
- Provide detailed error messages and suggestions for failed key validation
- Integrate parallel testing approach from PR #5147 with frontend filtering

Fixes the core issue where users entering incorrect API keys would be redirected
away from setup screen, now they stay on setup with clear error feedback.
- Replace problematic key generation that could result in null or duplicate keys
- Use fallback key pattern: message.id || `message-${index}-${role}`
- Ensures unique keys across all message components to prevent React warnings
- Fixes: 'Encountered two children with the same key, null' warning

This resolves the React warning that appeared when switching between providers
or when messages lacked proper IDs.
- Update Anthropic model from claude-3-haiku-20240307 to claude-3-opus-20240229
- Update OpenRouter model from anthropic/claude-3-haiku to anthropic/claude-3-opus
- Update success messages to reflect Claude Opus selection
- Provides users with the most capable Claude model by default

This gives users access to Claude 4's enhanced capabilities for better
code generation, analysis, and problem-solving performance.
- Change Anthropic model to claude-sonnet-4-0 (correct model identifier)
- Update OpenRouter model to anthropic/claude-sonnet-4-0
- Update success messages to reflect Claude Sonnet selection
- Ensures compatibility with the latest Claude Sonnet 4 model

This uses the proper model identifier for Claude Sonnet 4 as specified.
- Add OpenAI provider handling to smart workarounds for both Ollama fallback and 404 cases
- Set default OpenAI model to gpt-4.1 for optimal performance
- Configure OPENAI_API_KEY, GOOSE_PROVIDER=openai, GOOSE_MODEL=gpt-4.1
- Add success message: 'Configured OpenAI with GPT-4.1 model'
- Ensures OpenAI keys get proper configuration even when backend detection fails

Now all major providers (OpenAI, Anthropic, OpenRouter) have smart workarounds
with high-quality default models: GPT-4.1, Claude Sonnet 4, and Claude Sonnet 4 via OpenRouter.
…0-5147

* 'main' of github.com:block/goose: (243 commits)
  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)
  fix: use a lock to ensure only need to run tunnel just in case multiple go… (#5885)
  Feat: Added custom headers and toggle keyring CLI options (#5017)
  Feat/automatic update installation (#5345)
  fix: Added "Merged consecutive assistant messages" to the acceptable issues for moim injection check (#5933)
  fix: anthropic provider model fetching (#5932)
  [MCP-UI] add CSP for images to proxy HTML (#5931)
  fix: correct typo in blog post (AIMDOEL -> AIMODEL) (#5902)
  feat: @goose in terminal (native terminal support) (#5887)
  docs: adding AI-friendly features (#5918)
  Blog/advent of ai announcement (#5917)
  Extension selector behind ALPHA flag (#5892)
  blog: typo fixes (#5896)
  blog: fixing img url (#5895)
  blog: MCPs for Developers (#5884)
  docs: Extension Manager MCP (#5883)
  ...

# Conflicts:
#	crates/goose-server/src/routes/config_management.rs
#	crates/goose/src/providers/mod.rs
#	ui/desktop/openapi.json
#	ui/desktop/src/api/sdk.gen.ts
#	ui/desktop/src/api/types.gen.ts
#	ui/desktop/src/components/ProgressiveMessageList.tsx
@zanesq zanesq changed the title Zane/onboarding detect provider Onboarding detect provider Dec 2, 2025
return (
<div
key={message.id && `${message.id}-${message.content.length}`}
key={message.id || `message-${index}-${message.role || 'unknown'}`}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

unrelated fix from @spencrmartin "Resolve React key uniqueness warning in ProgressiveMessageList" looks legit to me

Copy link
Collaborator

Choose a reason for hiding this comment

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

shouldn't that just be message.id?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

id can be undefined based on the type.. I think its leftover from when we used to modify messages front end. I'll leave it for now until we can come back to the type and check that its now enforced everywhere. Updated to use message timestamp also.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It needs to be optional since messages only get their id once they get written to the database. if we want to keep this in, why the double || and the appending of the message role?

Copy link
Collaborator Author

@zanesq zanesq Dec 5, 2025

Choose a reason for hiding this comment

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

Got it, I simplified it to key={message.id ?? `msg-${index}-${message.created}`} in the latest commit 👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@zanesq zanesq changed the title Onboarding detect provider Onboarding detect provider from api key Dec 2, 2025
@zanesq zanesq requested a review from michaelneale December 2, 2025 23:16
@zanesq zanesq marked this pull request as ready for review December 2, 2025 23:16
@michaelneale
Copy link
Collaborator

@zanesq lovely - do you think we can as part of this enhance this so it could even detect from (interactive?) env that it has a named key (when I tried that, when it does work, is just delightful as zero config) - if you like I can look at ways of layering it on with this (as it would require some hand testing to make sure it works packaged vs justfile style) - ie use the -l style shell, sniff out 3 of the common key styles and suggest it automatically? (I can add it to this one if you like - or if you would rather a follow on LMK)

@michaelneale michaelneale self-assigned this Dec 2, 2025
@zanesq
Copy link
Collaborator Author

zanesq commented Dec 2, 2025

@michaelneale sure please go ahead, this branch is fine.. it sounds like a great enhancement and you would be better suited to test it etc. Can always follow up after this merges if you think it will take a while.

* 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)
@michaelneale
Copy link
Collaborator

@zanesq a good start but 2 missing things we need, and I think this should be a priority PR:

1 - it isn't configuring goose properly - extensions completely empty, even built in ones, so something about how it bootstraps is a very different path:

image

2 - when it did set it up, say with anthropic, it just picked what i thought was a random model (ie haiku) - not sure of logic of that but should have a recommended one for each maybe? (not sure how that works).

Otherwise - I really like this, could be a great improvement IMO, just those 2

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.

Can we revert this to my original implementation? #5147

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.

some more things I spotted

@zanesq
Copy link
Collaborator Author

zanesq commented Dec 5, 2025

@michaelneale good catch I was able to replicate the missing extensions issue intermittently using a custom GOOSE_PATH_ROOT both with the api key detection and with a custom provider, and the extensions show on refresh sometimes and after re-launching. I think its a pre-existing issue since we didn't touch any related config code in this branch and we aren't doing anything special other than what we did before by loading the provider and model and redirecting to home. I will look into that in another PR.

Regarding the preferred model choice, agreed it would be nice addition. I will look into that in a separate PR. Let me know if you have some ideas of how we could do that. Since it would need to be a hard coded list that might not match user preferences I think the best option would be to show a model chooser dialog when onboarding after selecting a provider?

@zanesq zanesq merged commit 1db4070 into main Dec 5, 2025
17 checks passed
@zanesq zanesq deleted the zane/onboarding-detect-provider branch December 5, 2025 21:06
zanesq added a commit that referenced this pull request Dec 5, 2025
Co-authored-by: spencrmartin <[email protected]>
Co-authored-by: Michael Neale <[email protected]>
@DOsinga
Copy link
Collaborator

DOsinga commented Dec 5, 2025

Regarding the preferred model choice, agreed it would be nice addition. I will look into that in a separate PR. Let me know if you have some ideas of how we could do that. Since it would need to be a hard coded list that might not match user preferences I think the best option would be to show a model chooser dialog when onboarding after selecting a provider?

the detection code does return a list of models already, so it shouldn't be too hard to show that in a dropdown after detection maybe?

@zanesq
Copy link
Collaborator Author

zanesq commented Dec 5, 2025

Yeah was thinking not difficult just where it would fit in the flow, should probably do it for configure providers also not just the auto key detection so basically after any provider is configured for the first time.. will see what I can come up with real quick

katzdave added a commit that referenced this pull request Dec 5, 2025
…nses-streaming

* 'main' of github.com:block/goose:
  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)
@zanesq
Copy link
Collaborator Author

zanesq commented Dec 5, 2025

@DOsinga I think it works well! #6005

tlongwell-block added a commit that referenced this pull request Dec 7, 2025
* origin/main:
  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)
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
zanesq added a commit that referenced this pull request Dec 9, 2025
* 'main' of github.com:block/goose:
  gov: new LF Projects LLC section (#6027)
  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)
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.

5 participants