Skip to content

acp: load configured extensions and refactor tests#6803

Merged
michaelneale merged 1 commit intomainfrom
acp-refactor
Jan 29, 2026
Merged

acp: load configured extensions and refactor tests#6803
michaelneale merged 1 commit intomainfrom
acp-refactor

Conversation

@codefromthecrypt
Copy link
Collaborator

Summary

This allows ACP to read and use the global extensions defined in goose config file. As these are global, they are loaded only once during initialization, not per session, the same way builtins are.

This also opportunistically refactors tests for the upcoming #6305 (providers), so that the diff there is less huge. Notably the tests now check the session_id more strictly and allow exact re-use between client (acp provider) and server (goose acp) modes.

Type of Change

  • Bug fix
  • Refactor / Code quality

AI Assistance

  • This PR was created or reviewed with AI assistance

Testing

New integration test that writes a config file for an MCP server and proves it was loaded as an extension by using it.

let mcp = McpFixture::new(expected_session_id.clone()).await;

let config_yaml = format!(
"extensions:\n lookup:\n enabled: true\n type: streamable_http\n name: lookup\n description: Lookup server\n uri: \"{}\"\n",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is the new test

Signed-off-by: Adrian Cole <adrian@tetrate.io>
@codefromthecrypt codefromthecrypt marked this pull request as ready for review January 29, 2026 07:42
Copilot AI review requested due to automatic review settings January 29, 2026 07:42
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 updates ACP so it loads globally configured extensions from the goose config file and refactors ACP server tests into reusable fixtures, improving session ID validation and enabling testing of configured extensions.

Changes:

  • Introduced get_extensions_map_with_config / get_enabled_extensions_with_config to read extensions from an arbitrary Config instance, and used this in the ACP server to load configured extensions from config.yaml.
  • Refactored ACP server tests into shared fixtures (fixtures and common_tests modules), adding a ClientToAgentSession abstraction, permission handling helpers, and a new run_configured_extension integration test.
  • Renamed GooseAcpConfig to AcpServerConfig and extended ACP server initialization to load both builtins and configured extensions while wiring through access to the PermissionManager.

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
crates/goose/src/config/extensions.rs Adds config-parameterized extension loading helpers so non-global configs (like ACP’s config dir) can resolve enabled extensions consistently.
crates/goose-mcp/.gooseignore Introduces a default .gooseignore template for the goose-mcp crate with sensible secret/env ignores.
crates/goose-acp/.gooseignore Same .gooseignore template added for the goose-acp crate to protect env and secrets files.
crates/goose-acp/src/server.rs Renames GooseAcpConfig to AcpServerConfig, exposes permission_manager, and on startup now reads config.yaml in config_dir to load enabled extensions in addition to builtins.
crates/goose-acp/tests/server_test.rs Simplifies ACP server tests to thin wrappers around shared common_tests helpers using run_test and ClientToAgentSession.
crates/goose-acp/tests/fixtures/mod.rs Builds reusable test infrastructure (OpenAI mocks, MCP fixtures, permission mapping, ACP in-process server spawning, and a generic Session trait plus run_test harness).
crates/goose-acp/tests/fixtures/server.rs Implements ClientToAgentSession using SACP, tracking notifications, permissions, and session IDs for reuse across tests.
crates/goose-acp/tests/common_tests/mod.rs Provides shared test flows (basic completion, MCP HTTP server, builtins+MCP, permission persistence, and configured extension loading) parameterized by session type.
crates/goose-acp/Cargo.toml Adds async-trait as a dev-dependency for async test trait implementations.
clippy-baselines/too_many_lines.txt Updates Clippy baseline to acknowledge an additional long function in the security module.
Cargo.lock Updates lockfile to include async-trait for the goose-acp crate.
Comments suppressed due to low confidence (1)

crates/goose-acp/src/server.rs:57

  • Renaming the public GooseAcpConfig struct to AcpServerConfig without a type alias or deprecated wrapper is a breaking API change for any external users of goose_acp::server, so consider either adding a pub type GooseAcpConfig = AcpServerConfig; for backward compatibility or documenting this as an intentional breaking change in the crate's release notes.
pub struct AcpServerConfig {
    pub provider: Arc<dyn goose::providers::base::Provider>,
    pub builtins: Vec<String>,
    pub data_dir: std::path::PathBuf,
    pub config_dir: std::path::PathBuf,
    pub goose_mode: goose::config::GooseMode,
}

@zhixiongdu027
Copy link

I've also encountered the issue where the global configuration doesn't take effect in ACP mode. Please help move this forward as soon as possible. Thanks!

Copy link
Collaborator

@michaelneale michaelneale left a comment

Choose a reason for hiding this comment

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

LGTM - did you mean to add to the too_many_lines.txt

@michaelneale
Copy link
Collaborator

merging as has hit me and others.

@michaelneale michaelneale merged commit b0c6373 into main Jan 29, 2026
24 checks passed
@michaelneale michaelneale deleted the acp-refactor branch January 29, 2026 23:39
zanesq added a commit that referenced this pull request Jan 30, 2026
* 'main' of github.com:block/goose:
  Fix: Small update UI settings prompt injection (#6830)
  Remove autogenerated .gooseignore files that don't belong in repo (#6824)
  Fix case-insensitive matching for builtin extension names (#6825)
  docs: cli newline keybinding (#6823)
  Update version to 1.22.0 (#6821)
  Refactor: move persisting extension to session outside of route (#6685)
  acp: load configured extensions and refactor tests (#6803)
  docs: usage data collection (#6822)
  feat: platform extension migrator + code mode rename (#6611)
  feat: CLI flag to skip loading profile extensions (#6780)
lifeizhou-ap added a commit that referenced this pull request Feb 2, 2026
* main:
  fix: fixed the broken release (#6887)
  feat: Streamable HTTP transport for ACP + goose-acp usage (#6741)
  Add Laminar for Observability (#6514)
  Missed a couple of places that hard code J for the newline key (#6853)
  fix(ui): preserve working directory when creating new chat (#6789)
  blog: add 5 tips for building MCP Apps that work (#6855)
  docs: session isolation (#6846)
  upgrade react and electron to latest (#6845)
  Fix: Small update UI settings prompt injection (#6830)
  Remove autogenerated .gooseignore files that don't belong in repo (#6824)
  Fix case-insensitive matching for builtin extension names (#6825)
  docs: cli newline keybinding (#6823)
  Update version to 1.22.0 (#6821)
  Refactor: move persisting extension to session outside of route (#6685)
  acp: load configured extensions and refactor tests (#6803)
Tyler-Hardin pushed a commit to Tyler-Hardin/goose that referenced this pull request Feb 11, 2026
Signed-off-by: Adrian Cole <adrian@tetrate.io>
Tyler-Hardin pushed a commit to Tyler-Hardin/goose that referenced this pull request Feb 11, 2026
Signed-off-by: Adrian Cole <adrian@tetrate.io>
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.

3 participants