feat(slimrpc): add SlimRPC transport support to a2acli - #102
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces configuration file support (.a2a.yaml) for the CLI, adds a new slimrpc transport feature, and refactors tenant handling by managing it within the client. Feedback on these changes highlights a compilation error due to a non-exhaustive match in Transport::protocol, a missing registration of SlimRpcTransportFactory in resolve_client, an absolute local path in Cargo.toml that will break external builds, and an opportunity to optimize request cloning in A2AClient when no tenant is specified.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| #[cfg(feature = "slimrpc")] | ||
| Slimrpc, |
There was a problem hiding this comment.
Adding the Slimrpc variant to Transport under #[cfg(feature = "slimrpc")] will cause a compilation error when the slimrpc feature is enabled, because the protocol method on Transport (defined right below) does not exhaustively match the new variant.
Please update Transport::protocol to handle the Slimrpc variant:
impl Transport {
fn protocol(self) -> &'static str {
match self {
Transport::Jsonrpc => TRANSPORT_PROTOCOL_JSONRPC,
Transport::HttpJson => TRANSPORT_PROTOCOL_HTTP_JSON,
#[cfg(feature = "slimrpc")]
Transport::Slimrpc => TRANSPORT_PROTOCOL_SLIMRPC,
}
}
}| #[cfg(feature = "slimrpc")] | ||
| use a2a_slimrpc; |
There was a problem hiding this comment.
The a2a_slimrpc dependency is imported here, but the SlimRpcTransportFactory is never registered on the A2AClientFactory in resolve_client. Without registering the factory, the CLI will not be able to negotiate or use the slimrpc transport, even if --transport slimrpc is specified.
Please update resolve_client to register the factory when the slimrpc feature is enabled:
let mut builder = A2AClientFactory::builder().preferred_bindings(preferred);
#[cfg(feature = "slimrpc")]
{
let slimrpc_factory = a2a_slimrpc::SlimRpcTransportFactory::from_slim_config().await?;
builder = builder.register(Arc::new(slimrpc_factory));
}| pbjson-build = "0.9" | ||
| protoc-bin-vendored = "3" | ||
| slim_bindings = { package = "agntcy-slim-bindings", version = "1.3.0" } | ||
| slim_bindings = { package = "agntcy-slim-bindings", path = "/Users/sambetts/Work/src/github.com/agntcy/slim-bindings/rust", version = "2.0.0-alpha.2" } |
There was a problem hiding this comment.
The dependency slim_bindings uses an absolute local path (/Users/sambetts/...). This will break builds on other environments and CI/CD pipelines. Please remove the local path or use a relative path if it is part of the same repository.
| slim_bindings = { package = "agntcy-slim-bindings", path = "/Users/sambetts/Work/src/github.com/agntcy/slim-bindings/rust", version = "2.0.0-alpha.2" } | |
| slim_bindings = { package = "agntcy-slim-bindings", version = "2.0.0-alpha.2" } |
| let patched = SendMessageRequest { | ||
| tenant: self.tenant.clone(), | ||
| ..req.clone() | ||
| }; | ||
| let result = self.transport.send_message(¶ms, &patched).await; |
There was a problem hiding this comment.
To avoid unnecessary cloning and allocations (especially for requests with large payloads), only clone and patch the request when self.tenant is Some.
let result = match &self.tenant {
Some(tenant) => {
let mut patched = req.clone();
patched.tenant = Some(tenant.clone());
self.transport.send_message(¶ms, &patched).await
}
None => self.transport.send_message(¶ms, req).await,
};3347eaa to
b07eef8
Compare
- Rename binary from `a2acli` to `a2a` - Replace `--base-url` global with positional `<agent-ref>` on every command - Group task commands under `task get/list/cancel/subscribe` - Replace `card`/`extended-card` with `discover [--extended]` - Add `-o/--output pretty|json` replacing `--compact` - Replace `--binding` with `--transport` (repeatable, ordered); defaults to jsonrpc + http-json so SLIMRPC is opt-in even when compiled with the feature - Show meaningful parse error when agent card JSON is invalid - Support local file paths as agent references Signed-off-by: Sam Betts <1769706+Tehsmash@users.noreply.github.com>
- Add hierarchical .a2a.yaml discovery (CWD up to home dir) that configures transports, bearer_token, headers, and output format; CLI flags always take precedence over config file values - Remove --tenant CLI flag and config option; tenant is now automatically injected by A2AClient from the selected AgentInterface, so callers never need to set it manually - A2AClient.with_tenant() stores the interface tenant and overrides the tenant field in every outgoing request, making it non-overridable by the caller Signed-off-by: Sam Betts <1769706+Tehsmash@users.noreply.github.com>
Signed-off-by: Sam Betts <1769706+Tehsmash@users.noreply.github.com>
Signed-off-by: Sam Betts <1769706+Tehsmash@users.noreply.github.com>
…nfig fallback and async file I/O - A2AClient methods now take request structs by value (mut req) and assign req.tenant in place, eliminating the struct-spread clone on every call - find_config_file() falls back to checking the home directory directly when CWD is outside the home directory tree (e.g. /tmp) - resolve_agent_card uses tokio::fs::read_to_string instead of the blocking std::fs variant - Remove the forward slimrpc stub from parse_binding_str; it will be re-added in the follow-up slimrpc PR Signed-off-by: Sam Betts <1769706+Tehsmash@users.noreply.github.com>
Gate file I/O imports and config-loading functions with #[cfg(not(test))] since they are only reachable from the non-test code path, and move the mut rebinding of cli inside that block to silence the unused_mut warning. Signed-off-by: Sam Betts <1769706+Tehsmash@users.noreply.github.com>
Replace the stub a2acli/README.md with a complete command reference covering all subcommands, global options, config file, auth, output formats, and transport negotiation. Document the AGENT_REF resolution rules (base URL, direct .json URL, local file path) throughout. Add a2acli-skill/SKILL.md as a concise skill reference for AI-assisted CLI use. Update the root README's CLI section to use current command names and link to the full reference. Signed-off-by: Sam Betts <1769706+Tehsmash@users.noreply.github.com>
9c20a5e to
7ef636b
Compare
… slimrpc support Add `SlimRpcTransport::from_slim_config(remote)` and `SlimRpcTransportFactory::from_slim_config()` async constructors that discover `slim.yaml` walking up from CWD (with env var overrides), initialize the global SLIM service, connect to the node, and subscribe the local app in one call. Update a2acli with a new `slimrpc` feature (off by default) that adds `--binding slimrpc` and `--slimrpc-target <org/ns/agent>` flags. When used, the CLI bypasses HTTP agent-card discovery and builds a `SlimRpcTransport` directly from the slim.yaml config. Point the workspace `slim_bindings` dep at the local `agntcy-slim-bindings` path (feat/hierarchical-slim-config-loading branch) and pin transitive `agntcy-slim-*` packages to the versions in slim-bindings' own lockfile to avoid version conflicts. Signed-off-by: Sam Betts <1769706+Tehsmash@users.noreply.github.com>
…parse entry Signed-off-by: Sam Betts <1769706+Tehsmash@users.noreply.github.com>
Signed-off-by: Sam Betts <1769706+Tehsmash@users.noreply.github.com>
Pass `None` to `load_slim_config()` following the updated signature that adds an optional explicit config path. Also fix the pre-existing type mismatch where `SlimAppHandle` was returned instead of `Arc<App>` by extracting `.app` from the handle. Signed-off-by: Sam Betts <1769706+Tehsmash@users.noreply.github.com>
- Register SlimRpcTransportFactory with the a2acli client builder when --enabled-binding slimrpc is requested, so the factory can actually dispatch SLIMRPC transports instead of falling through to JSONRPC - Return conn_id from create_app_from_slim_config and pass it through to SlimRpcTransport/SlimRpcTransportFactory via new_with_connection - Add info-level tracing in create_from_card to log candidate bindings, each attempt, and the selected binding Signed-off-by: Sam Betts <1769706+Tehsmash@users.noreply.github.com>
SlimRpcTransport and SlimRpcTransportFactory now expose a fluent builder via ::builder(). Setters: with_app, with_remote, with_connection, with_service, with_channel (transport only), with_slim_config. The with_slim_config path reuses a provided service if with_service was also called, otherwise creates a new one. Service lifetime is tied to the transport/factory via Arc, so connections are not dropped prematurely. Also fix pre-existing clippy::uninlined_format_args warnings across a2a, a2a-client, a2a-server, and examples. Signed-off-by: Sam Betts <1769706+Tehsmash@users.noreply.github.com>
b07eef8 to
afe7092
Compare
- Pin slim_rpc, slim_config, slim_service, slim_datapath, slim_auth to the feat/hierarchical-slim-config-loading branch - Add slim-config feature to slim_service dep - Move slim_config from a2a-slimrpc dev-deps to regular deps - Remove unused slim_bindings dep from a2acli - Add tracing_subscriber::fmt::init() to a2acli main - fmt: reflow long lines in a2acli config/tests and a2a-client Signed-off-by: Sam Betts <1769706+Tehsmash@users.noreply.github.com>
7ef636b to
56e65fa
Compare
Summary
Adds SlimRPC as an opt-in transport in the `a2a` CLI, building on the transport/config redesign in #101.
Changes
`a2acli` SlimRPC integration
`a2a-slimrpc` config discovery and connection ID
Transport binding observability
Depends on
PR #101 (`feat/cli-redesign-1929`) — this branch is stacked on top of it.