Add Amp as a known ACP provider#1312
Conversation
Register amp-acp as a known ACP provider alongside Goose, Claude Code, and Codex. ## Changes - `desktop/src-tauri/src/managed_agents/discovery.rs` — Add `amp` to `KNOWN_ACP_RUNTIMES` with command `amp-acp` and avatar URL - `crates/buzz-acp/src/config.rs` — Add `amp-acp` to `default_agent_args()` match arm - `crates/buzz-acp/README.md` — Add "Running with Amp" section with install and usage instructions; list amp-acp in supported agents Revives block#208 by akuttig-block. Co-authored-by: Robby Cohen <robbyc@squareup.com> Signed-off-by: Robby Cohen <robbyc@squareup.com>
The Amp ACP provider entry added 22 lines to discovery.rs, pushing it past the existing 1043-line override. Bump to 1065 to match actual size. File remains queued to split per the existing comment. Co-authored-by: Robby Cohen <robbyc@squareup.com> Signed-off-by: Robby Cohen <robbyc@squareup.com>
| KnownAcpRuntime { | ||
| id: "amp", | ||
| label: "Amp", | ||
| commands: &["amp-acp"], |
There was a problem hiding this comment.
Can you also add amp-acp to the desktop-side default_agent_args() match lower in this file? crates/buzz-acp/src/config.rs was updated so the standalone harness treats Amp as zero-arg, but managed desktop agents have a separate normalization path. Keeping both lists in sync avoids preserving a stray acp arg if one is ever passed through for this runtime.
| mcp_hooks: false, | ||
| underlying_cli: Some("amp"), | ||
| cli_install_commands: &[], | ||
| adapter_install_commands: &["npm install -g amp-acp"], |
There was a problem hiding this comment.
Before we expose this as a one-click known runtime install, can we clarify that amp-acp is a third-party npm adapter (or switch to an official Sourcegraph/Amp adapter if one exists)? The package depends on @ampcode/cli/@ampcode/sdk, but the adapter/repo itself appears to be individually maintained. Since this command installs latest globally, the docs/UI should make that trust boundary explicit, or pin a known-good version.
|
@codex please review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 165dcef604
ℹ️ 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".
| avatar_url: AMP_AVATAR_URL, | ||
| mcp_command: None, | ||
| mcp_hooks: false, | ||
| underlying_cli: Some("amp"), |
There was a problem hiding this comment.
Don't require Amp CLI when only amp-acp is needed
The added runtime is treated as cli_missing whenever amp-acp is installed but the amp CLI is not, because classify_runtime requires underlying_cli to resolve before returning available. That blocks the new provider from useAvailableAcpRuntimes, even for users following the new README path of installing amp-acp and authenticating with AMP_API_KEY; the amp-acp docs also describe a standalone/no-CLI setup path (amp-acp --setup or env auth). Set this to None unless Buzz truly requires the Amp CLI, otherwise a working adapter is hidden from the create-agent flow.
Useful? React with 👍 / 👎.
Register amp-acp as a known ACP provider alongside Goose, Claude Code, and Codex.
Revives #208 (originally by @akuttig-block), updated for the current
KnownAcpRuntimestruct shape.Changes
desktop/src-tauri/src/managed_agents/discovery.rs— AddamptoKNOWN_ACP_RUNTIMESwith commandamp-acp, avatar URL, and all required struct fieldscrates/buzz-acp/src/config.rs— Addamp-acptodefault_agent_args()match arm so it runs with no extra argscrates/buzz-acp/README.md— Add "Running with Amp" section; list amp-acp in supported agentsInstall
Usage
The desktop app auto-discovers
amp-acpif it's on PATH.