Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6044d67109
ℹ️ 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".
crates/goose/src/config/base.rs
Outdated
|
|
||
| let secrets = if env::var("GOOSE_DISABLE_KEYRING").is_ok() | ||
| || keyring_disabled_in_config(&config_path) | ||
| || keyring_disabled_in_config(&user_config_path) |
There was a problem hiding this comment.
With mergeable configs, GOOSE_DISABLE_KEYRING can now be set in /etc/goose/config.yaml, but Config::default still only checks ~/.config/goose/config.yaml before choosing SecretStorage. In environments that rely on a global config, this ignores the configured backend policy and initializes keyring mode unexpectedly, which can change secret handling behavior at runtime.
Useful? React with 👍 / 👎.
crates/goose/src/config/base.rs
Outdated
| let mut config_paths = vec![system_config_path()]; | ||
| if let Some(defaults) = bundled_defaults_path() { | ||
| config_paths.push(defaults); |
There was a problem hiding this comment.
The merge order loads /etc/goose/config.yaml first and defaults.yaml second, and load() applies last-wins merging for normal keys. When defaults.yaml is present, bundled defaults can overwrite administrator settings from the system config (including extension fields), so global config no longer acts as a reliable baseline unless users re-override it.
Useful? React with 👍 / 👎.
DOsinga
added
the
needs_human
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e73446f85
ℹ️ 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".
| let secrets = if no_secrets_config | ||
| .get_param::<serde_json::Value>("GOOSE_DISABLE_KEYRING") | ||
| .is_ok() |
There was a problem hiding this comment.
Config::default now picks file-based secret storage whenever get_param("GOOSE_DISABLE_KEYRING") returns any value, because the condition only checks .is_ok(). That means configs like GOOSE_DISABLE_KEYRING: false (or any non-missing value) still disable keyring and route secrets to secrets.yaml, which is a behavioral and security regression compared to boolean semantics. The check should deserialize/interpret the value as a boolean truthy flag instead of testing mere presence.
Useful? React with 👍 / 👎.
2 participants
This PR introduces mergeable config files, so goose will pull configuration from multiple sources: /etc/goose/config.yaml, and ~/.config/goose/config.yaml. You can have an extension defined in the global config and enable/disable it in your user config.
Also, remove the backup/restore logic.