Skip to content

Conversation

@jamadeo
Copy link
Collaborator

@jamadeo jamadeo commented Oct 22, 2025

This changes the provider settings modals to:

  • show optional config parameters in a default-hidden section
  • don't resubmit values the user doesn't change
  • show a masked version of secret values instead of nothing
  • be simpler

OpenAI looks like

Screenshot 2025-10-22 at 11 22 59 PM Screenshot 2025-10-22 at 11 23 28 PM

@jamadeo jamadeo changed the title Set optional config params, and don't overwrite unset secrets feat/fix: set optional config params, and don't overwrite unset secrets Oct 22, 2025
@jamadeo jamadeo marked this pull request as ready for review October 23, 2025 03:25
@jamadeo jamadeo requested review from DOsinga and zanesq October 23, 2025 03:26
showDeleteConfirmation={showDeleteConfirmation}
onConfirmDelete={handleConfirmDelete}
onCancelDelete={() => {}}
onCancelDelete={() => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to change the provider's "active" state? Will this mark providers as inactive when the user simply cancels the confirmation dialog?

Copy link
Collaborator

Choose a reason for hiding this comment

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

activeProvider is a shadow variable that is supposed to be true only for the provider that is the current one. I think we can safely get rid of the entire thing and just at the top read in what the current provider is and then everywhere where we check activeProvider, just check if that provider is the current provider. we're really playing fast and loose with setting and unsetting this as is

const [showDeleteConfirmation, setShowDeleteConfirmation] = useState(false);
const [isActiveProvider, setIsActiveProvider] = useState(false); // New state for tracking active provider
const [requiredParameters, setRequiredParameters] = useState<ConfigKey[]>([]); // New state for tracking active provider
const [isActiveProvider, setIsActiveProvider] = useState(false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

oh, is this no longer a new state?

showDeleteConfirmation={showDeleteConfirmation}
onConfirmDelete={handleConfirmDelete}
onCancelDelete={() => {}}
onCancelDelete={() => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

activeProvider is a shadow variable that is supposed to be true only for the provider that is the current one. I think we can safely get rid of the entire thing and just at the top read in what the current provider is and then everywhere where we check activeProvider, just check if that provider is the current provider. we're really playing fast and loose with setting and unsetting this as is

type={type}
className={cn(
'flex h-9 w-full rounded-md border focus:border-border-strong hover:border-border-strong bg-background-default px-3 py-1 text-base transition-colors file:border-0 file:bg-transparent file:text-sm file:font-medium file:text-foreground placeholder:text-textPlaceholder focus-visible:outline-none disabled:cursor-not-allowed disabled:opacity-50 md:text-sm',
'flex h-9 w-full rounded-md border focus:border-border-strong hover:border-border-strong bg-background-default px-3 py-1 text-base transition-colors file:border-0 file:bg-transparent file:text-sm file:font-medium file:text-foreground placeholder:text-textPlaceholder placeholder:font-light focus-visible:outline-none disabled:cursor-not-allowed disabled:opacity-50 md:text-sm',
Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm?

@DOsinga DOsinga merged commit ad42693 into main Oct 30, 2025
14 checks passed
@DOsinga DOsinga deleted the jackamadeo/optional-provider-gui branch October 30, 2025 20:04
zanesq added a commit that referenced this pull request Oct 30, 2025
* 'main' of github.com:block/goose: (81 commits)
  nextcamp - fix session resume when navigating back to chat in sidebar (#5370)
  feat/fix: set optional config params, and don't overwrite unset secrets (#5325)
  Stringly typed config (#5463)
  Fix: Compaction client <-> server sync  (#5481)
  docs: recipe activity parameter substitution (#5462)
  only run fork on branch PRs (#5461)
  docs: video on goose with apify mcp (#5472)
  Clear windows and fix build failure (#5452)
  Add menu option for setting window always on top (#5429)
  Delete environment variable (#5479)
  chore: upgrade rmcp to 0.8.3 (#5458)
  docs: add "Building Custom Tools and Extensions for Goose" (#5469)
  Doc (Blog): Managing goose Configurations Across Multiple Projects (#5467)
  apify doc fix (#5460)
  Stream token usage on every agent message (#5342)
  rpm install in /opt/Goose to avoid conflicts with chrome-sandbox (#5421)
  Don't disable extensions after they fail to activate in new chat session (#5464)
  Add OTLP logs layer (#5386)
  openapi to locust load test generator recipe (#5447)
  technical debt tracker recipe (#5451)
  ...

# Conflicts:
#	ui/desktop/src/components/ChatInput.tsx
michaelneale added a commit that referenced this pull request Oct 31, 2025
* main: (45 commits)
  Change Recipes Test Script (#5457)
  Goose recover (#5450)
  don't start the default provider (#5351)
  keep the order of keys in config.yaml (#5468)
  Removed drafts and agentIsReady in ChatInput (#5366)
  nextcamp - fix session resume when navigating back to chat in sidebar (#5370)
  feat/fix: set optional config params, and don't overwrite unset secrets (#5325)
  Stringly typed config (#5463)
  Fix: Compaction client <-> server sync  (#5481)
  docs: recipe activity parameter substitution (#5462)
  only run fork on branch PRs (#5461)
  docs: video on goose with apify mcp (#5472)
  Clear windows and fix build failure (#5452)
  Add menu option for setting window always on top (#5429)
  Delete environment variable (#5479)
  chore: upgrade rmcp to 0.8.3 (#5458)
  docs: add "Building Custom Tools and Extensions for Goose" (#5469)
  Doc (Blog): Managing goose Configurations Across Multiple Projects (#5467)
  apify doc fix (#5460)
  Stream token usage on every agent message (#5342)
  ...
katzdave added a commit that referenced this pull request Nov 3, 2025
* 'main' of github.com:block/goose: (61 commits)
  [Autovisualiser] remove unnecessary content from mermaid HTML template (#5505)
  Improve subagents docs (#5484)
  FIX: prefer linux in WSL and add INSTALL_OS override for CLI (#5215)
  Propagate session ID in LLM and MCP requests (#5165)
  feat: YT Short for Canva MCP + goose (#5495)
  Change Recipes Test Script (#5457)
  Goose recover (#5450)
  don't start the default provider (#5351)
  keep the order of keys in config.yaml (#5468)
  Removed drafts and agentIsReady in ChatInput (#5366)
  nextcamp - fix session resume when navigating back to chat in sidebar (#5370)
  feat/fix: set optional config params, and don't overwrite unset secrets (#5325)
  Stringly typed config (#5463)
  Fix: Compaction client <-> server sync  (#5481)
  docs: recipe activity parameter substitution (#5462)
  only run fork on branch PRs (#5461)
  docs: video on goose with apify mcp (#5472)
  Clear windows and fix build failure (#5452)
  Add menu option for setting window always on top (#5429)
  Delete environment variable (#5479)
  ...
fbalicchia pushed a commit to fbalicchia/goose that referenced this pull request Nov 7, 2025
BlairAllan pushed a commit to BlairAllan/goose that referenced this pull request Nov 29, 2025
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.

4 participants