Skip to content

Stringly typed config #5463

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
jamadeo merged 6 commits into from
Oct 30, 2025
Merged

Stringly typed config #5463

jamadeo merged 6 commits into from
Oct 30, 2025

Conversation

@jamadeo
Copy link
Collaborator

@jamadeo jamadeo commented Oct 29, 2025 •
edited
Loading

  • remove some .get() calls to config
  • make the router thing a boolean (it was a string)

@jamadeo jamadeo force-pushed the stringly-typed-config branch from 1d70297 to 06fb5bf Compare October 30, 2025 00:06
@jamadeo jamadeo requested a review from Copilot October 30, 2025 00:06
Copilot AI reviewed Oct 30, 2025
View reviewed changes
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 refactors configuration management by introducing a type-safe GooseMode enum and streamlining the config setter API. It replaces string-based mode comparisons with strongly-typed enum variants and updates all setters to accept generic serializable values instead of requiring pre-serialized serde_json::Value instances.

  • Introduced GooseMode enum to replace string-based mode handling
  • Updated set_param and set_secret methods to accept generic serializable types
  • Added declare_param! macro to generate type-safe getter/setter methods for common config parameters

Reviewed Changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
crates/goose/src/config/goose_mode.rs New file defining the GooseMode enum with Auto, Approve, SmartApprove, and Chat variants
crates/goose/src/config/base.rs Updated config methods to use generics and added macro-generated getters/setters for GOOSE_MODE, GOOSE_PROVIDER, and GOOSE_MODEL
crates/goose/src/config/mod.rs Exported the new GooseMode enum
crates/goose/src/agents/agent.rs Replaced string-based mode handling with GooseMode enum
crates/goose/src/permission/permission_inspector.rs Updated to use GooseMode instead of strings
crates/goose/src/providers/ollama.rs Updated mode checking to use GooseMode enum
crates/goose/src/providers/claude_code.rs Updated mode checking and removed obsolete test
crates/goose/src/providers/githubcopilot.rs Simplified secret setting calls
crates/goose/src/config/signup_tetrate/mod.rs Replaced serde_json::Value calls with direct value passing
crates/goose/src/config/signup_openrouter/mod.rs Replaced serde_json::Value calls with direct value passing
crates/goose-cli/src/commands/configure.rs Updated to use GooseMode enum and simplified config setter calls
crates/goose-cli/src/session/mod.rs Updated mode validation to use GooseMode::try_from
Multiple other files Updated config setter calls to pass values directly instead of wrapping in Value

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jamadeo jamadeo requested a review from DOsinga October 30, 2025 02:22
DOsinga
DOsinga approved these changes Oct 30, 2025
View reviewed changes
crates/goose/src/agents/recipe_tools/dynamic_task_tools.rs
pub fn should_enabled_subagents(model_name: &str) -> bool {
let config = crate::config::Config::global();
let is_autonomous = config.get_param("GOOSE_MODE").unwrap_or("auto".to_string()) == "auto";
let is_autonomous = config.get_goose_mode().unwrap_or(GooseMode::Auto) == GooseMode::Auto;
Copy link
Collaborator

@DOsinga DOsinga Oct 30, 2025

Choose a reason for hiding this comment

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

here and elsewhere; having a typed get_goose_mode is of course so much better, but should we add a default value to the pattern? we never read goose mode without defaulting to auto if it is not set

Copy link
Collaborator Author

@jamadeo jamadeo Oct 30, 2025

Choose a reason for hiding this comment

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

yeah, we should do that. every config needs some kind of default value (it might be None of course)

@jamadeo jamadeo merged commit 2970b5f into main Oct 30, 2025
14 checks passed
@jamadeo jamadeo deleted the stringly-typed-config branch October 30, 2025 19:13
zanesq added a commit that referenced this pull request Oct 30, 2025
@zanesq
Merge branch 'main' of github.com:block/goose into zane/remove-drafts
37d18a4 
* '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
@michaelneale
Merge branch 'main' into micn/goose-mobile-access
9ad4c89 
* 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
@katzdave
Merge branch 'main' of github.com:block/goose into error-proxy-2
23518fd 
* '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)
  ...
@github-actions github-actions bot mentioned this pull request Nov 5, 2025
Merged
fbalicchia pushed a commit to fbalicchia/goose that referenced this pull request Nov 7, 2025
@jamadeo @fbalicchia
Stringly typed config (block#5463)
c69cb59 
Signed-off-by: fbalicchia <[email protected]>
BlairAllan pushed a commit to BlairAllan/goose that referenced this pull request Nov 29, 2025
@jamadeo @BlairAllan
Stringly typed config (block#5463)
efa3a26 
Signed-off-by: Blair Allan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@DOsinga DOsinga DOsinga approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jamadeo @DOsinga