Skip to content

Conversation

@cbruyndoncx
Copy link
Contributor

Pull Request Description

Summary

  • Fixes a bug where custom providers added via the desktop UI (or saved JSON files) appeared multiple times in the provider list and showed incomplete data in the Configure modal. This change ensures custom providers are registered once in the provider registry with correct metadata (including config keys), and the server returns the registry metadata to the UI rather than re-parsing files.

Problem

  • The server route that returned provider metadata re-read custom provider JSON files and manually constructed ProviderMetadata objects while the provider registry was also registering those same providers. This caused:
    • Duplicate provider cards in the desktop provider grid.
    • Incomplete or inconsistent provider configuration metadata shown in the Configure modal (UI relies on metadata.config_keys).
  • The registry-registered custom providers did not include the per-provider config_keys (API key env var and base URL) — instead they used the base provider's config_keys, leaving the UI without the correct fields.

What I changed

  • crates/goose/src/providers/provider_registry.rs
    • register_with_name now accepts an explicit config_keys: Vec and uses it to build ProviderMetadata for custom providers (instead of defaulting to base provider config keys). This allows each custom provider to expose the correct configuration keys to the UI.
  • crates/goose/src/config/custom_providers.rs
    • register_custom_providers now constructs appropriate config keys for each custom provider:
      • ConfigKey for the custom provider API key environment name (from the JSON file).
      • ConfigKey for CUSTOM_PROVIDER_BASE_URL with the provider's base_url as default.
    • Passes the config_keys into register_with_name when registering the provider with the registry.
  • crates/goose-server/src/routes/config_management.rs
    • providers() route simplified: instead of re-reading and manually parsing custom provider JSON files, it now returns provider metadata from the central provider registry (get_providers()) and maps that into the API response.
    • This removes duplication and ensures the UI sees the exact metadata the registry uses.

Why this fixes the bug

  • There is now a single source of truth for provider metadata (the ProviderRegistry). Because custom providers are registered with correct config_keys, the UI gets consistent and complete metadata and renders a single card per provider with a complete Configure modal.

Testing / How to verify
Manual steps (desktop app):

  1. Start the server + desktop app (or run the server API endpoints locally).
  2. Create a custom provider using the desktop UI (Add Custom Provider) or by dropping a JSON file in the custom_providers directory that matches the CustomProviderConfig structure.
  3. Refresh providers in the UI.
  4. Verify:
    • Only one card appears for the custom provider.
    • Clicking Configure shows the expected configuration fields:
      • The API key field should use the provider's api_key_env name.
      • The Base URL field should be present and prefilled with the provider's base_url value (or listed as a default).
    • Adding/removing a custom provider and refreshing should update the list with no duplicates.
  5. Optionally, call GET /config/providers and confirm the provider appears once and has the correct config_keys in the JSON response.

Automated / build checks:

  • The repository builds successfully (cargo build).
  • No change to external APIs (only internal metadata handling). All changes are internal to provider registration and server provider listing.

Files changed

  • crates/goose/src/providers/provider_registry.rs
    • register_with_name signature and usage
  • crates/goose/src/config/custom_providers.rs
    • register_custom_providers now builds and passes config_keys
  • crates/goose-server/src/routes/config_management.rs
    • providers() now uses provider registry rather than re-parsing JSON files

Migration / compatibility notes

  • Internal signature of register_with_name changed (internal to goose crate). No external API change expected for users of the binary.
  • If other code in the project manually called register_with_name, those call sites would need to pass config_keys. (The project currently only registers custom providers from the custom_providers module.)

Follow-ups / suggestions

  • Add unit/integration tests to:
    • Assert register_custom_providers produces ProviderMetadata with expected config_keys.
    • Assert GET /config/providers returns no duplicates after adding custom providers.
  • Consider expanding CustomProviderConfig to allow explicit config_key customization in JSON (if you want a different key than CUSTOM_PROVIDER_BASE_URL or more keys).
  • Add a small CHANGELOG entry describing the bug fix and user-facing effects (duplicate cards, configure modal).

Notes for reviewers

  • Review provider_registry.register_with_name to ensure the config_keys usage aligns with expectations for other provider types.
  • Review register_custom_providers for any additional fields to expose in config_keys (headers, timeout, etc.), depending on how much you want surfaced in the UI

@cbruyndoncx cbruyndoncx force-pushed the fix-duplicate-provider-in-ui branch from a03dac5 to 71d137c Compare September 4, 2025 12:37
@cbruyndoncx
Copy link
Contributor Author

This solved the deduplication in the UI, but have not tested end to end yet. If i understand things well, if i use the cli on windows, it would use the same goosed (goose-server), so my issue with wrong keys reported earlier might be fixed ... to be continued ...

@cbruyndoncx
Copy link
Contributor Author

The deduplication is fixed, but the actual keys are not loaded properly, debugging further ...

…pandable toast with request+response; refresh providers
…tead of returning Err so frontend gets provider response details
@cbruyndoncx
Copy link
Contributor Author

Changes included in #4557

@cbruyndoncx cbruyndoncx closed this Sep 8, 2025
@cbruyndoncx cbruyndoncx deleted the fix-duplicate-provider-in-ui branch September 8, 2025 11:05
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.

1 participant