fix: respect disabled builtin extensions (developer) at session start

[michaelneale]

Summary

Fixes #10221. A user who disables the Developer extension (toggle off in Settings, enabled: false in config.yaml) still gets its tools (shell, edit, write, tree, read_image) loaded into every new chat, and shell commands execute. Any mitigation built on disabling Developer is silently void.

Fix

Skip a builtin when the config has an explicit enabled: false entry for it:

for builtin in &self.builtins {
    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);
}

Docs

Also corrects the stale Developer tool table in documentation/docs/mcp/developer-mcp.md (it listed text_editor/analyze/screen_capture/image_processor; the platform extension actually ships shell, write, edit, tree, read_image, and codebase analysis is now the separate Analyze extension).

Testing

• cargo test -p goose --lib config::extensions — added unit tests for the new helpers (missing entry, default-on platform ext, explicit disable/enable)
• cargo build -p goose, cargo clippy -p goose --lib, cargo fmt — clean

[github-actions]

Documentation preview deployed: https://pr-10223.goose-pr-previews-poc.pages.dev

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 082b9f2d20

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[DOsinga]

Thanks for addressing the default-off case. I still think the current shape has a design smell though.

is_builtin_disabled_by_user is named in terms of user intent, but it cannot really know that intent because the config has already gone through read migrations that synthesize entries for platform extensions. The implementation then has to infer intent from enabled: false plus PLATFORM_EXTENSIONS[name].default_enabled, which is why the doc comment needs to explain this in terms of migrations, default_enabled, code_execution, and developer. That feels like the abstraction is leaking rather than just needing a shorter comment.

I think the distinction we actually care about is the source of the builtin request: builtins goose adds by default for ACP/serve should respect config disabled state, while builtins explicitly requested by a caller should be loaded because they were explicit. Could we model that distinction directly, for example with separate default vs explicit builtin lists or a small enum, instead of trying to recover it from platform defaults after config migration?

That would make the selection policy more direct and avoid a helper whose behavior is defined by unrelated migration details.

[michaelneale]

@DOsinga yeah I am not sure what we want as end state, what expectation is. Is the idea that we have a config yaml same as today where people say something is on or off, and if it is off, it stays off? as in an ACP world that seems less clear to me.

[lifeizhou-ap]

Hi @michaelneale,

Thanks for the fix!

With the current fix, it has different behaviour for the goose serve from the other cli. For example,

goose serve --with-builtin developer

if the developer is disabled, the session won't have developer extension. This fixes the bug, but the behaviour is opposite to our --with-builtin argument. In cli, when we pass --with-builtin the extension will be included in the session no matter of it is enable or disabled in the config file.

So I think @DOsinga's suggestion makes sense. Instead of pass builtin to goose serve, we can pass

#[derive(Clone, Debug, Default)]
pub struct AcpBuiltinSelection {
pub defaults: Vec,
pub explicit: Vec,
}

For goose serve:

let builtin_selection = if builtins.is_empty() {
AcpBuiltinSelection {
defaults: vec!["developer".to_string()],
explicit: vec![],
}
} else {
AcpBuiltinSelection {
defaults: vec![],
explicit: builtins,
}
};

For goose acp:

let builtin_selection = AcpBuiltinSelection {
defaults: vec![],
explicit: builtins,
};

and replace selected_builtin_extensions with something like below

fn selected_builtin_extensions(
config: &Config,
default_builtins: &[String],
explicit_builtins: &[String],
) -> Vec {
let mut extensions = Vec::new();

for builtin in default_builtins {
    if configured_enabled_state(config, builtin) != Some(false) {
        push_or_replace_extension(
            &mut extensions,
            builtin_to_extension_config(builtin),
        );
    }
}

for builtin in explicit_builtins {
    push_or_replace_extension(
        &mut extensions,
        builtin_to_extension_config(builtin),
    );
}

extensions

}

We can remove this fallback logic in cli, I think this is the original confusing and smell part prior to your fix.

let builtins = if builtins.is_empty() {
        vec!["developer".to_string()]
    } else {
        builtins
    };

Happy to have a sync chat for more clarification!

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 18a23e1e47

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Borrow the extension name for the config check

This passes the String returned by ExtensionConfig::name() to configured_enabled_state, whose second parameter is &str, so the goose crate no longer compiles when this helper is built. Borrow the returned name (or bind it first) before doing the disabled-state comparison.

Useful? React with 👍 / 👎.

[michaelneale]

Maybe in closer to desired state now ready for review again