mirror of
https://github.com/aaif-goose/goose.git
synced 2026-07-03 14:10:03 +02:00
fix: don't force-load explicitly disabled builtin extensions
The ACP server unconditionally pushed every builtin (e.g. developer) into each new session before consulting the extensions config. A user who disabled developer in Settings still got shell/edit/write/tree/read_image handed to the model, because the config-derived list only *adds* enabled extensions and never removes the force-added builtin. Skip a builtin when the config has an explicit enabled: false entry for it. Missing entries still load (fresh installs / default-on bundled extensions behave as before), so this only changes the explicitly-disabled case. Adds config::extensions::configured_enabled_state / is_extension_explicitly_disabled and unit tests.
This commit is contained in:
@@ -16,7 +16,9 @@ use crate::agents::{
|
||||
Agent, AgentConfig, ExtensionConfig, ExtensionLoadResult, GoosePlatform, SessionConfig,
|
||||
};
|
||||
use crate::config::base::CONFIG_YAML_NAME;
|
||||
use crate::config::extensions::get_enabled_extensions_with_config;
|
||||
use crate::config::extensions::{
|
||||
get_enabled_extensions_with_config, is_extension_explicitly_disabled,
|
||||
};
|
||||
use crate::config::paths::Paths;
|
||||
use crate::config::permission::PermissionManager;
|
||||
use crate::config::{Config, GooseMode};
|
||||
@@ -1028,7 +1030,11 @@ impl GooseAcpAgent {
|
||||
) -> Result<Vec<ExtensionConfig>, agent_client_protocol::Error> {
|
||||
let mut extensions = Vec::new();
|
||||
for builtin in &self.builtins {
|
||||
push_or_replace_extension(&mut extensions, builtin_to_extension_config(builtin));
|
||||
let builtin_config = builtin_to_extension_config(builtin);
|
||||
if is_extension_explicitly_disabled(config, &builtin_config.name()) {
|
||||
continue;
|
||||
}
|
||||
push_or_replace_extension(&mut extensions, builtin_config);
|
||||
}
|
||||
|
||||
if let Some(recipe_extensions) = recipe_extensions {
|
||||
|
||||
@@ -199,6 +199,27 @@ pub fn is_extension_enabled(key: &str) -> bool {
|
||||
extensions.get(key).map(|e| e.enabled).unwrap_or(false)
|
||||
}
|
||||
|
||||
/// Returns the configured enabled state for an extension, or `None` when the
|
||||
/// extension has no entry in the config at all.
|
||||
///
|
||||
/// This lets callers distinguish "not configured" (e.g. a fresh install where a
|
||||
/// bundled builtin should still load by default) from "explicitly turned off".
|
||||
pub fn configured_enabled_state(config: &Config, name: &str) -> Option<bool> {
|
||||
let extensions = get_extensions_map_with_config(config);
|
||||
let key = name_to_key(name);
|
||||
extensions
|
||||
.values()
|
||||
.find(|entry| entry.config.name() == name)
|
||||
.or_else(|| extensions.get(&key))
|
||||
.map(|entry| entry.enabled)
|
||||
}
|
||||
|
||||
/// Returns true only when an extension has an explicit config entry that is
|
||||
/// disabled. Missing entries return false so default-on builtins keep loading.
|
||||
pub fn is_extension_explicitly_disabled(config: &Config, name: &str) -> bool {
|
||||
matches!(configured_enabled_state(config, name), Some(false))
|
||||
}
|
||||
|
||||
pub fn get_enabled_extensions() -> Vec<ExtensionConfig> {
|
||||
get_all_extensions()
|
||||
.into_iter()
|
||||
@@ -664,4 +685,39 @@ extensions:
|
||||
other_keys
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_configured_enabled_state_unknown_extension_is_none() {
|
||||
let (config, _config_file, _secrets_file) = test_config("");
|
||||
|
||||
assert_eq!(
|
||||
configured_enabled_state(&config, "not_a_real_extension"),
|
||||
None
|
||||
);
|
||||
assert!(!is_extension_explicitly_disabled(
|
||||
&config,
|
||||
"not_a_real_extension"
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_default_enabled_platform_extension_is_not_explicitly_disabled() {
|
||||
let (config, _config_file, _secrets_file) = test_config("");
|
||||
|
||||
assert_eq!(configured_enabled_state(&config, "developer"), Some(true));
|
||||
assert!(!is_extension_explicitly_disabled(&config, "developer"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_configured_enabled_state_reflects_saved_entry() {
|
||||
let (config, _config_file, _secrets_file) = test_config("");
|
||||
set_extension_with_config(&config, builtin_entry("developer", false));
|
||||
|
||||
assert_eq!(configured_enabled_state(&config, "developer"), Some(false));
|
||||
assert!(is_extension_explicitly_disabled(&config, "developer"));
|
||||
|
||||
set_extension_enabled_with_config(&config, "developer", true);
|
||||
assert_eq!(configured_enabled_state(&config, "developer"), Some(true));
|
||||
assert!(!is_extension_explicitly_disabled(&config, "developer"));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user