Feat/a2a v1 protocol compat (fixes issue #263)#272
Conversation
Signed-off-by: luigisaetta <luigi.saetta@oracle.com>
Signed-off-by: luigisaetta <luigi.saetta@oracle.com>
Signed-off-by: luigisaetta <luigi.saetta@oracle.com>
There was a problem hiding this comment.
Spent some time on this one Luigi, ended up running the canonical a2a-sdk client against your branch to see how the wire actually behaves. Short version: implementation is in good shape, found one wire bug that the reference client catches (separate review with the anchor).
Few things I checked first:
hatch run checkis clean (format, lint, mypy all happy)hatch run test-fastshows 4931 passing. The 7 failures I saw also fail on main, they're anoracledbnot being in the hatch env thing, not yours- Cross-referenced your
protocol_v1.pyandspec_v1.pyline by line against the v1.0 spec. Method names, enum names (ROLE_, TASK_STATE_), version negotiation, error codes, SSE framing, all look right. Particularly liked the auto-detect that routes v1 method names even when the client forgets to set theA2A-Versionheader.
I also wrote 20 integration tests against your branch (real httpx hitting a real uvicorn, plus two real-model gpt-4o-mini round-trips for sanity). All green. Gist here: https://gist.github.com/fede-kamel/bc0f1ea7f6f1bb23c418aa9108e13f40
Main thing I'd push on before merge is the lack of any live v1 tests in the repo. The TCK MUST claim in the description is believable but nothing in CI actually backs it up. Happy to push the gist in as a follow-up commit on your branch, open a stacked PR, or leave it for you to take and run with, whatever you prefer.
A few smaller notes inline. None of those are blocking on their own.
There was a problem hiding this comment.
Quick follow-up. Got curious about how a non-locus v1 client would behave against this server, so I pulled in the canonical a2a-sdk (v1.0.3, the official Python reference) and pointed it at a locus instance running this branch.
Results:
✓ ClientFactory.create_from_url() resolved /.well-known/agent-card.json
✓ send_message → TASK_STATE_COMPLETED after 5 stream events
✗ get_task: Message type "lf.a2a.v1.Task" has no field named "task" at "Task".
✓ list_tasks → totalSize=1 (page returned 1)
✓ cancel_task on terminal → TaskNotCancelableError (correct)
The get_task failure is real. The SDK's JSON-RPC transport does json_format.ParseDict(json_rpc_response.result, Task()), so it parses result directly as a Task. But locus wraps it: result: {"task": {...}}. Same thing happens on cancel_task (would have shown up if I'd tested the non-terminal path).
SendMessage and ListTasks are fine because their response types actually want a wrapped/structured shape.
My own integration tests didn't catch this because I asserted the wrapped form too (result["task"]["id"]), basically codified the same bug. My bad. Easy fix once I knew what to look for.
Proposed fix is 2 lines server side + 2 assertion updates in the unit tests. Wrote it up with before/after proof in the gist, full diff in fix.patch. Re-ran everything with the fix applied:
hatch run checkclean- 162 unit tests pass
- My 20 integration tests pass (with the assertion updates)
- a2a-sdk reference client interop goes from 1 failure to all green
On the broader "is locus a good v1 citizen" question: a2a-sdk 1.0 is functionally the only v1 cross-SDK peer right now. Both Strands and Google ADK are still pinned to a2a-sdk <0.4 so they haven't migrated yet. With this fix locus passes the only meaningful interop bar that exists today.
Posting the inline anchors as a separate review so the suggestion blocks attach to the right lines.
There was a problem hiding this comment.
Inline anchors for the two-line fix from the last review. Each suggestion block is the actual change, so it's one-click to apply if you're happy with it.
Gist with the full reproduction and before/after proof: https://gist.github.com/fede-kamel/bc0f1ea7f6f1bb23c418aa9108e13f40
Signed-off-by: luigisaetta <luigi.saetta@oracle.com>
25e45c8 to
6cb2ef3
Compare
|
Addressed the A2A v1 review feedback and kept the PR scoped to the compatibility work. Changes made:
Validation:
|
Signed-off-by: luigisaetta <luigi.saetta@oracle.com>
Signed-off-by: luigisaetta <luigi.saetta@oracle.com>
6cb2ef3 to
c8cadcb
Compare
Signed-off-by: Federico Kamelhar <federico.kamelhar@oracle.com>
GitHub secret scanning flagged the literal "v1-real-model-secret" in the real_model_server fixture as a possible credential. It's purely a loopback test fixture, not a credential, but the string itself trips the heuristic. Rename to a less-suspicious literal and add a gitleaks:allow marker matching the sibling fixture style. Signed-off-by: Federico Kamelhar <federico.kamelhar@oracle.com>
2c8c0c1 to
81e4a5a
Compare
fede-kamel
left a comment
There was a problem hiding this comment.
Approving. All CI green on the latest head (81e4a5a):
- Lint + typecheck (3.11, 3.14)
- Tests (3.11, 3.12, 3.13, 3.14)
- Pre-commit hooks
- DCO
- CI Success aggregate
- OCA
Cross-SDK conformance verified end-to-end: the canonical a2a-sdk 1.0.3 reference client round-trips against this server cleanly (SendMessage, GetTask, ListTasks, CancelTask all green; the wrap bug from the earlier review is fixed). The new tests/integration/test_a2a_v1_live.py (20 tests including two real-model gpt-4o-mini round-trips) locks the conformance contract in for future regressions.
Open follow-up: #273 (move the unrelated _oracle_pool_cache.py + memory/backends/adapters.py cleanups into a separate PR). Not blocking this merge.
Nice work Luigi.
Summary
This PR fixes issue #263 by adding A2A v1 protocol compatibility to Locus.
It introduces an A2A v1 compatible server implementation, including:
tasks/listsupportIt also adds an A2A v1 compatible client
Compatibility
The Locus A2A server has been updated to satisfy the MUST level requirements checked by the A2A Technology Compatibility Kit, TCK
This ensures that Locus can interoperate correctly with A2A v1 clients and related tooling.
Tests
This PR adds 20 A2A/v1-related unit test functions covering protocol v1 model conversion, JSON-RPC handling, task listing, streaming, client interoperability, and router behavior.
These tests are located in
The new tests cover the v1 compatibility behaviour introduced by this PR.
The full test suite was also run with hatch:
4929 tests passed
Additional changes
This PR also includes:
Validation
End to end compatibility was verified by developing:
The client and server were tested together successfully, confirming that Locus is compatible with the A2A v1 protocol expectations.