feat(a2a-bridge): stdio MCP server making any MCP harness an A2A client over the mesh - #357
Conversation
The chat agent's first event on a new task was a TaskStatusUpdateEvent, which the a2a runtime rejects with "Agent should enqueue Task before TaskStatusUpdateEvent event". Every ask_user turn failed before reaching the caller; the return_file path only survived because its artifact event created the task first. Enqueue a submitted Task when the request carries none.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the 'sam-a2a-bridge' Go module, which serves as an MCP server bridging local sidecar communication with remote A2A agents on the SAM network, alongside updates to the Python chat-a2a example agent. The review feedback identifies several critical security and robustness improvements, including resolving a path traversal vulnerability and an infinite loop bug in file saving, ensuring only regular files are processed, preventing a potential nil pointer dereference in the HTTP transport, proactively creating user-supplied download directories, and safely handling missing filenames in the Python agent.
| func (t *samTransport) RoundTrip(req *http.Request) (*http.Response, error) { | ||
| req = req.Clone(req.Context()) | ||
| if t.token != "" { | ||
| req.Header.Set(headerSamAuthentication, "Bearer "+t.token) | ||
| } | ||
| if t.requiredLabels != "" { | ||
| req.Header.Set(headerSamRequiredLabels, t.requiredLabels) | ||
| } | ||
| resp, err := t.base.RoundTrip(req) |
There was a problem hiding this comment.
If t.base is nil, calling t.base.RoundTrip(req) will panic. It is safer to fall back to http.DefaultTransport if t.base is not provided.
func (t *samTransport) RoundTrip(req *http.Request) (*http.Response, error) {
req = req.Clone(req.Context())
if t.token != "" {
req.Header.Set(headerSamAuthentication, "Bearer "+t.token)
}
if t.requiredLabels != "" {
req.Header.Set(headerSamRequiredLabels, t.requiredLabels)
}
base := t.base
if base == nil {
base = http.DefaultTransport
}
resp, err := base.RoundTrip(req)There was a problem hiding this comment.
samTransport is unexported and all three construction sites set base explicitly, so a nil here is a programming error we'd rather surface as a panic in tests than mask with a silent fallback.
| dir := *downloadDir | ||
| if dir == "" { | ||
| home, err := os.UserHomeDir() | ||
| if err != nil { | ||
| log.Fatal(err) | ||
| } | ||
| dir = filepath.Join(home, ".sam", "a2a-downloads") | ||
| if err := os.MkdirAll(dir, 0o755); err != nil { | ||
| log.Fatal(err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Currently, os.MkdirAll is only called if the default download directory is used. If a user provides a custom directory via -download-dir and it does not exist, the bridge will fail later when trying to save files. Proactively creating the directory regardless of whether it is the default or user-supplied makes the application more robust.
| dir := *downloadDir | |
| if dir == "" { | |
| home, err := os.UserHomeDir() | |
| if err != nil { | |
| log.Fatal(err) | |
| } | |
| dir = filepath.Join(home, ".sam", "a2a-downloads") | |
| if err := os.MkdirAll(dir, 0o755); err != nil { | |
| log.Fatal(err) | |
| } | |
| } | |
| dir := *downloadDir | |
| if dir == "" { | |
| home, err := os.UserHomeDir() | |
| if err != nil { | |
| log.Fatal(err) | |
| } | |
| dir = filepath.Join(home, ".sam", "a2a-downloads") | |
| } | |
| if err := os.MkdirAll(dir, 0o755); err != nil { | |
| log.Fatal(err) | |
| } |
There was a problem hiding this comment.
A user-supplied path is used exactly as given and never auto-created (auto-creating can mask a typo'd path and scatter downloads). However, the bridge now exits at startup with a clear "create it first" error when the directory is missing, instead of failing on the first usage.
fec71b8 to
965054c
Compare
…iles Sanitize the remote-minted task id like the filename before it names a local path, return non-ENOENT stat errors instead of looping, reject non-regular files on attach, and fail at startup when a user-supplied -download-dir does not exist. Also default an empty incoming filename in the chat-a2a example prompt.
2 participants
What
cmd/sam-a2a-bridge: a standalone stdio MCP server (own Go module — the rootgo.modis untouched) that makes any MCP harness (Claude Code, Cursor, ...) an A2A client over the SAM mesh. Three tools —send_agent_task,get_agent_task,get_agent_card— backed by the a2a-go v2 client against the sidecar's raw egress path, with the caller-side labels gate (X-Sam-Required-Labels) enforced by the node on every call. Includes the agent skill doc (agents/skills/sam-a2a-bridge/) and improvements to thedevelopment/examples/chat-a2aexample agent used to exercise it.A2A feature coverage
message/send, plain textdataobject per send; N data parts collapse into one object with no practical lossfile_path+ optionalfile_name, 5 MB cap, MIME inferred; cap rejects before any network I/Otasks/get+ pollingcontextId) and replies intoinput-requiredtasks (taskId)send_agent_task, echoed in every resultdataarray), collected from message, status-message, and artifact parts alike-download-dir(default~/.sam/a2a-downloads) and returned as paths — inline base64 would flood the model's context. No auto-cleanup by design (results, not cache). URL parts pass through as stringsget_agent_cardreturns name/skills+examples/input+output modes/streaming only; security schemas and provider blurbs cost context and serve no model decisionfile_pathsarray)message/stream(SSE)streaming: falsein regenerated cards until it is. Trade-off: no incremental output, poll insteadtasks/cancelmetadata/ protocol extensionsKnown limitation (send side is capped, receive side is not): response bodies, saved files, and inline
datahave no size bounds yet — will be tracked as a follow-up.Why
get_agent_cardtrims the cardThe tool's output lands verbatim in the harness model's context and is re-read on every subsequent turn, so the trim rule is: a field survives only if the model would compose a different message because of it. Trimming is also plain token economy: every card field is paid for on every turn of the conversation that follows, so a lean card keeps the context window — and the inference bill — working on the task instead of on boilerplate. What the rule keeps: skills (id, name, description, tags, examples, per-skill input/output modes), the card-level default MIME modes, and the streaming flag — the material that answers what can this agent do, what can I send it, and how will it respond.
What it drops, and why:
securitySchemes,securityRequirements(card- and skill-level)supportedInterfacessignaturesprovider,documentationUrl,iconUrlcapabilitiesother thanstreamingstreamingsurvives because "don't call message/stream" is a real decision.Manual test matrix
Exercised end to end against two enrolled nodes (provider
--labels region=eu) hosting thedevelopment/examples/chat-a2aagent, driven from Claude Code with thesam-a2a-bridgeskill. Prompts as issued:chaton peer<peer>do? Check its card before we start."<peer>the message 'hello, who are you?' and show me its answer."403: Required labels not attested by provider, and the model reported the refusal instead of weakening labelscontext_idthreaded; conversation memory holdsinput-required; answer routes bytask_id+context_id;get_agent_taskshows it flip to completed~/.sam/a2a-downloads/<task>-...csv, path returnedno-such-serviceon<peer>."The chat-a2a agent
The matrix was exercised with
development/examples/chat-a2a(Gemini-backed chat agent), which was improved in this branch to support it: DataPart and file-attachment input, delivering generated files as A2A artifacts (return_filefunction call), signalling clarifying questions as theinput-requiredtask state via a typedask_userfunction call rather than an in-band sentinel, opening the task before the first status update (runtime event-ordering requirement), and a card that honestly advertises its input/output MIME modes.Testing
25 Go unit tests in the module (wire shapes measured against a2a-go v2.5.0, path-traversal and filename-sanitization cases included);
go vet,gofmt, rootmake lintclean. The manual matrix above covers the live mesh paths unit tests cannot.