Skip to content

[codex] fix declarative provider auth updates and Ollama Cloud model selection#8499

Closed
AiRC-ai wants to merge 5 commits intoaaif-goose:mainfrom
AiRC-ai:codex/ollama-cloud-provider-fixes
Closed

[codex] fix declarative provider auth updates and Ollama Cloud model selection#8499
AiRC-ai wants to merge 5 commits intoaaif-goose:mainfrom
AiRC-ai:codex/ollama-cloud-provider-fixes

Conversation

@AiRC-ai
Copy link
Copy Markdown

@AiRC-ai AiRC-ai commented Apr 12, 2026

Summary

This stacks the declarative-provider auth update flow with two follow-up fixes needed to make the Ollama Cloud configuration path work end to end.

Changes included:

  • allow fixed declarative providers to save auth updates from the provider settings modal
  • persist auth overrides for fixed declarative providers instead of treating them like fully editable custom providers
  • await provider refresh before opening model selection after an update, so the model picker uses fresh provider state
  • skip canonical filtering for ollama_cloud so the full live cloud model list is shown instead of an over-filtered subset

Root cause

There were two separate issues in the provider flow:

  • fixed declarative providers could accept an API key in the UI, but the update path did not fully persist and reload those auth overrides in a way the rest of Goose treated as configured
  • after a successful update, the model picker could open against stale cached provider state, and Ollama Cloud's live model list was additionally being filtered too aggressively by canonical-model filtering

Impact

This makes the Ollama Cloud provider flow usable from the desktop settings UI:

  • Update Provider persists the API key for the fixed declarative provider
  • the follow-up model picker opens with refreshed provider state
  • the provider shows the live Ollama Cloud models instead of a nearly empty list

Validation

  • source bin/activate-hermit && cd ui/desktop && pnpm run typecheck
  • rebuilt and packaged the macOS desktop app locally with just package-ui
  • manually verified the desktop flow:
    • add/update Ollama Cloud API key
    • open model picker after update
    • see populated cloud models and select one successfully

Notes

The first two commits are the #8191-equivalent declarative-provider fixes on top of main; the last commit is the follow-up for the refreshed model picker and Ollama Cloud model list behavior.

Abhijay007 and others added 4 commits April 12, 2026 15:17
Signed-off-by: Abhijay Jain <Abhijay007j@gmail.com>
Signed-off-by: Abhijay Jain <Abhijay007j@gmail.com>
Signed-off-by: Ryan Cox <ryancox@GoDSyNcs-MBP.godsync.com>
Signed-off-by: Ryan Cox <ryancox@GoDSyNcs-MBP.godsync.com>
@AiRC-ai AiRC-ai marked this pull request as ready for review April 12, 2026 22:46
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0aea068501

ℹ️ 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".

Comment on lines +316 to +319
if custom_file.exists() {
std::fs::remove_file(custom_file)?;
}
None
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Persist disabled auth overrides for fixed providers

When requires_auth is turned off for a non-editable declarative provider, this path removes the shadow file and writes no replacement override, so the setting is not persisted. On the next reload, load_provider falls back to the bundled fixed config (which is requires_auth: true for providers like ollama_cloud), so users cannot actually keep auth disabled and the checkbox state snaps back after refresh.

Useful? React with 👍 / 👎.

Signed-off-by: Ryan Cox <ryancox@GoDSyNcs-MBP.godsync.com>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fac75a343d

ℹ️ 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".

Comment on lines +453 to +455
let (shadows, true_custom): (Vec<_>, Vec<_>) = custom_providers
.into_iter()
.partition(|c| fixed_names.contains(c.name.as_str()));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Stop treating fixed-name custom providers as auth shadows

This partition now reclassifies any custom provider whose name matches a fixed provider into the shadow path, where only requires_auth/api_key_env are applied and all other custom settings are discarded. Because generate_id creates IDs like custom_{display_name}, creating a provider named DeepSeek or Tensorix collides with bundled fixed IDs (custom_deepseek, custom_tensorix), so the saved custom api_url/models are ignored and the provider effectively becomes non-editable fixed config after refresh.

Useful? React with 👍 / 👎.

@lifeizhou-ap
Copy link
Copy Markdown
Collaborator

Thanks for the contribution, @AiRC-ai! We noticed you have 3 open PRs (#8497, #8499, #8500) opened around the same time — per our contribution guidelines we'd suggest focusing on one at a time so we can give it proper review. There are also a couple of unaddressed Codex review comments on this PR (one flags a real regression where custom providers whose name collides with a fixed provider would have their settings silently dropped). We're going to close this one for now — please feel free to reopen your top-priority PR with the Codex feedback addressed, and we'll take a look!

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