Initial integration test kit#445
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request establishes an Integration Test Kit (ITK) designed to validate the interoperability and compatibility of different A2A SDK implementations, specifically focusing on Go and Python agents. It introduces a standardized Highlights
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces an integration test kit (ITK) for verifying compatibility between Go and Python A2A SDKs. The implementation includes Go and Python agents, a protobuf-based instruction protocol, and a test runner script. The overall approach is solid, providing a good foundation for cross-SDK testing.
My review includes several suggestions:
- Critical: The Python agent's proto build script is broken, which corrupts a generated file and prevents the tests from running.
- Medium: The Go agent could be improved with graceful shutdown handling, similar to the Python agent. There's also some dead code to clean up. The Python test runner relies on
sys.pathmanipulation which should be avoided by improving the packaging. - Low: I've pointed out a typo in the README, some minor code style issues, an unused import, and opportunities to use more specific exception handling.
Once the critical build issue is resolved, this will be a valuable addition to the project.
|
|
||
| # Fix imports in generated gRPC file for local module usage | ||
| # (Standard protoc-gen-grpc-python behavior requires this for relative packages) | ||
| sed -i 'import instruction_pb2 as instruction__pb2/from . import instruction_pb2 as instruction__pb2/' pyproto/instruction_pb2_grpc.py |
There was a problem hiding this comment.
This sed command is syntactically incorrect. It's missing the s (substitute) command and proper delimiters, which has resulted in the pyproto/instruction_pb2_grpc.py file being corrupted.
To fix this, the command should use the s/find/replace/ syntax.
| sed -i 'import instruction_pb2 as instruction__pb2/from . import instruction_pb2 as instruction__pb2/' pyproto/instruction_pb2_grpc.py | |
| sed -i 's/import instruction_pb2 as instruction__pb2/from . import instruction_pb2 as instruction__pb2/' pyproto/instruction_pb2_grpc.py |
There was a problem hiding this comment.
has been addressed
| mport instruction_pb2 as instruction__pb2/from . import instruction_pb2 as instruction__pb2/ | ||
| # Generated by the gRPC Python protocol compiler plugin. DO NOT EDIT! | ||
| mport instruction_pb2 as instruction__pb2/from . import instruction_pb2 as instruction__pb2/ | ||
| """Client and server classes corresponding to protobuf-defined services.""" | ||
| mport instruction_pb2 as instruction__pb2/from . import instruction_pb2 as instruction__pb2/ | ||
| import grpc | ||
| mport instruction_pb2 as instruction__pb2/from . import instruction_pb2 as instruction__pb2/ | ||
| import warnings | ||
| mport instruction_pb2 as instruction__pb2/from . import instruction_pb2 as instruction__pb2/ | ||
|
|
||
| mport instruction_pb2 as instruction__pb2/from . import instruction_pb2 as instruction__pb2/ | ||
|
|
||
| mport instruction_pb2 as instruction__pb2/from . import instruction_pb2 as instruction__pb2/ | ||
| GRPC_GENERATED_VERSION = '1.78.1' | ||
| mport instruction_pb2 as instruction__pb2/from . import instruction_pb2 as instruction__pb2/ | ||
| GRPC_VERSION = grpc.__version__ | ||
| mport instruction_pb2 as instruction__pb2/from . import instruction_pb2 as instruction__pb2/ | ||
| _version_not_supported = False | ||
| mport instruction_pb2 as instruction__pb2/from . import instruction_pb2 as instruction__pb2/ | ||
|
|
||
| mport instruction_pb2 as instruction__pb2/from . import instruction_pb2 as instruction__pb2/ | ||
| try: | ||
| mport instruction_pb2 as instruction__pb2/from . import instruction_pb2 as instruction__pb2/ | ||
| from grpc._utilities import first_version_is_lower | ||
| mport instruction_pb2 as instruction__pb2/from . import instruction_pb2 as instruction__pb2/ | ||
| _version_not_supported = first_version_is_lower(GRPC_VERSION, GRPC_GENERATED_VERSION) | ||
| mport instruction_pb2 as instruction__pb2/from . import instruction_pb2 as instruction__pb2/ | ||
| except ImportError: | ||
| mport instruction_pb2 as instruction__pb2/from . import instruction_pb2 as instruction__pb2/ | ||
| _version_not_supported = True | ||
| mport instruction_pb2 as instruction__pb2/from . import instruction_pb2 as instruction__pb2/ | ||
|
|
||
| mport instruction_pb2 as instruction__pb2/from . import instruction_pb2 as instruction__pb2/ | ||
| if _version_not_supported: | ||
| mport instruction_pb2 as instruction__pb2/from . import instruction_pb2 as instruction__pb2/ | ||
| raise RuntimeError( | ||
| mport instruction_pb2 as instruction__pb2/from . import instruction_pb2 as instruction__pb2/ | ||
| f'The grpc package installed is at version {GRPC_VERSION},' | ||
| mport instruction_pb2 as instruction__pb2/from . import instruction_pb2 as instruction__pb2/ | ||
| + ' but the generated code in instruction_pb2_grpc.py depends on' | ||
| mport instruction_pb2 as instruction__pb2/from . import instruction_pb2 as instruction__pb2/ | ||
| + f' grpcio>={GRPC_GENERATED_VERSION}.' | ||
| mport instruction_pb2 as instruction__pb2/from . import instruction_pb2 as instruction__pb2/ | ||
| + f' Please upgrade your grpc module to grpcio>={GRPC_GENERATED_VERSION}' | ||
| mport instruction_pb2 as instruction__pb2/from . import instruction_pb2 as instruction__pb2/ | ||
| + f' or downgrade your generated code using grpcio-tools<={GRPC_VERSION}.' | ||
| mport instruction_pb2 as instruction__pb2/from . import instruction_pb2 as instruction__pb2/ | ||
| ) |
There was a problem hiding this comment.
has been addressed
| // Wrap instruction ba | ||
| } |
| func main() { | ||
| flag.Parse() | ||
|
|
||
| host := "127.0.0.1" | ||
| jsonRpcAddr := fmt.Sprintf("http://127.0.0.1:%d/jsonrpc", *httpPort) | ||
| grpcAddr := fmt.Sprintf("127.0.0.1:%d", *grpcPort) | ||
|
|
||
| skill := a2a.AgentSkill{ | ||
| ID: "itk_v03_proto_skill", | ||
| Name: "ITK v03 Proto Skill", | ||
| Description: "Handles raw byte Instruction protos in v03 subproject.", | ||
| Tags: []string{"proto", "v03", "itk"}, | ||
| Examples: []string{"Roll a dice", "Call another agent"}, | ||
| } | ||
|
|
||
| // go-sdk v.03 has support for JSONRPC and GRPC only | ||
| agentCard := &a2a.AgentCard{ | ||
| Name: "ITK v03 Agent", | ||
| Description: "Multi-transport agent supporting raw Instruction protos.", | ||
| URL: jsonRpcAddr, | ||
| Version: "0.3.0", | ||
| DefaultInputModes: []string{"text"}, | ||
| DefaultOutputModes: []string{"text"}, | ||
| Capabilities: a2a.AgentCapabilities{Streaming: false}, | ||
| Skills: []a2a.AgentSkill{skill}, | ||
| AdditionalInterfaces: []a2a.AgentInterface{ | ||
| {URL: grpcAddr, Transport: a2a.TransportProtocolGRPC}, | ||
| }, | ||
| } | ||
|
|
||
| executor := &V03AgentExecutor{} | ||
| requestHandler := a2asrv.NewHandler(executor, a2asrv.WithExtendedAgentCard(agentCard)) | ||
|
|
||
| // 1. JSON-RPC handler | ||
| jsonrpcHandler := a2asrv.NewJSONRPCHandler(requestHandler) | ||
| cardHandler := a2asrv.NewStaticAgentCardHandler(agentCard) | ||
|
|
||
| mux := http.NewServeMux() | ||
| mux.Handle(a2asrv.WellKnownAgentCardPath, cardHandler) | ||
| mux.Handle("/jsonrpc", jsonrpcHandler) | ||
|
|
||
| httpServer := &http.Server{ | ||
| Addr: host + ":" + fmt.Sprintf("%d", *httpPort), | ||
| Handler: mux, | ||
| } | ||
|
|
||
| // 2. gRPC handler | ||
| grpcServer := grpc.NewServer() | ||
| grpcHandler := a2agrpc.NewHandler(requestHandler) | ||
| grpcHandler.RegisterWith(grpcServer) | ||
|
|
||
| // 3. Run servers concurrently | ||
| g, _ := errgroup.WithContext(context.Background()) | ||
|
|
||
| g.Go(func() error { | ||
| log.Printf("Starting HTTP server on %s:%d (JSON-RPC)", host, *httpPort) | ||
| return httpServer.ListenAndServe() | ||
| }) | ||
|
|
||
| g.Go(func() error { | ||
| lis, err := net.Listen("tcp", host+":"+fmt.Sprintf("%d", *grpcPort)) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| log.Printf("Starting gRPC server on %s:%d", host, *grpcPort) | ||
| return grpcServer.Serve(lis) | ||
| }) | ||
|
|
||
| if err := g.Wait(); err != nil { | ||
| log.Fatalf("Server failed: %v", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
The Go agent currently doesn't handle signals (like SIGINT/SIGTERM) for graceful shutdown. When the test runner terminates it, it will be killed abruptly. This can be improved by adding signal handling to gracefully shut down the HTTP and gRPC servers. The Python agent implementation already does this, and it would be good for the Go agent to be similarly robust.
You could use the context from errgroup.WithContext to orchestrate the shutdown.
There was a problem hiding this comment.
has been addressed
| echo "Compiling protos from ../../../protos into pyproto/..." | ||
|
|
||
| # Create pyproto directory if it doesn't exist | ||
| mkdir -p pyproto |
There was a problem hiding this comment.
There was a problem hiding this comment.
has been addressed
| "agents/ag2", | ||
| "extensions/timestamp", | ||
| "extensions/traceability", | ||
| "itk/agents/python", |
There was a problem hiding this comment.
has been addressed
| ## 🚀 Getting Started | ||
|
|
||
| ### Prerequisites | ||
| - [uv](https://github.com/astral-sh/uv) for Python dependecy management. |
There was a problem hiding this comment.
has been addressed
| --go_opt=Minstruction.proto=itk/agents/go/v03/pb \ | ||
| --go_opt=paths=source_relative \ | ||
| ../../../protos/instruction.proto | ||
|
|
There was a problem hiding this comment.
has been addressed
| except Exception: | ||
| continue |
There was a problem hiding this comment.
will leave todo in the comment
| import base64 | ||
| import logging | ||
| import os | ||
| import signal |
There was a problem hiding this comment.
this has been addressed
2f34ca1 to
4991430
Compare
|
Cross-SDK compatibility tests are valuable beyond QA — they encode the behavioral contract at inter-agent boundaries. What you're testing here is effectively a behavioral spec: "given this input, this agent responds this way, regardless of whether it's Go or Python." That's exactly the signal a discovery layer needs — not just AgentCard declarations (which describe claimed capabilities), but verifiable behavioral records (which describe actual behavior under test conditions). A few thoughts from building Agenium (an agent registry + discovery layer): Discovery filter granularity: When an agent registers, it typically declares transport support (jsonrpc, grpc, http_json). Your cross-SDK test kit generates empirical data on which transport bindings actually pass — that's a tighter signal than the agent's own declaration. Discovery layers could consume this as a verified transport attestation, not just a field in the AgentCard. Cold-start trust: New agents in a network have no behavioral history. Integration test suites like this could become a bootstrap mechanism — run the standard suite, pass rate gets published, other agents can reference it at discovery time before trusting a first interaction. Test ownership at boundaries: In multi-agent systems, who owns the test at the interface? This PR implicitly says: "a neutral test runner verifies both sides." That's the right frame — the contract is between the callers and the protocol, not owned by either SDK. Nice work on making this SDK-agnostic. |
198ce1c to
9aa8eb6
Compare
639ce32 to
c28d221
Compare
herczyn
left a comment
There was a problem hiding this comment.
approving for merge, we should iterate further (adding 1.0 agents and running in github actions)
please address ai comments if they make sense
|
one more ask: let's move itk to be in the top folder, these samples are not specific to python |
fa3c9c6 to
3e74dca
Compare
Description
The pull request includes:
CONTRIBUTINGGuide.Fixes #<issue_number_goes_here> 🦕