feat: Support Message-only simplified execution without creating Task#956
feat: Support Message-only simplified execution without creating Task#956
Conversation
bartek-w
force-pushed
the
bartekw-on-message-unification
branch
2 times, most recently
from
4fe6f59 to
bd8917f
Compare
![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 significant updates to the A2A SDK, including a transition to Protobuf-based serialization, the addition of database migration tools, and support for A2A protocol v1.0. It also adds ITK integration test support and updates dependencies. I have reviewed the configuration files and identified several instances where lists of dependencies and exclusion patterns are not sorted alphabetically, which should be corrected to maintain consistency.
I am having trouble creating individual review comments. Click here to see my feedback.
.github/actions/spelling/excludes.txt (91-96)
The newly added exclusion patterns are not sorted alphabetically. Please sort them to maintain consistency with the rest of the file.
.jscpd.json (7)
The ignore list is not sorted alphabetically. Sorting it will make it easier to manage as the project grows.
pyproject.toml (16-19)
The dependencies list is not sorted alphabetically. Please sort them to maintain consistency.
pyproject.toml (110-112)
The dev dependency list is not sorted alphabetically. Please sort them to maintain consistency.
bartek-w
force-pushed
the
bartekw-on-message-unification
branch
from
bd8917f to
b5c3e78
Compare
🧪 Code Coverage (vs
|
| Base | PR | Delta | |
|---|---|---|---|
| src/a2a/server/agent_execution/active_task.py | 94.06% | 94.78% | 🟢 +0.72% |
| src/a2a/server/request_handlers/default_request_handler_v2.py | 92.05% | 92.24% | 🟢 +0.19% |
| src/a2a/server/tasks/task_manager.py | 98.29% | 98.32% | 🟢 +0.03% |
| Total | 92.33% | 92.38% | 🟢 +0.05% |
Generated by coverage-comment.yml
| if isinstance(event, Message): | ||
| break |
There was a problem hiding this comment.
This is an invalid state (wrong agent executor implementation), subscribe can happen only against an already running task, so we shouldn't encounter Message here.
| if isinstance(event, Task): | ||
| new_task = event | ||
| await self._task_manager.save_task_event( | ||
| new_task | ||
| ) | ||
| else: |
There was a problem hiding this comment.
If I run tests from #954 against a new executor history assertions fail with message being duplicated. The utility method we have new_task populates first user message in the history. I am not sure if we should rely on it or maybe avoid saving message with the same id?
Either way it's something that changed from the "legacy" executor.
(maybe it's a known issue as a part of enqueue Task TODO)
2 participants
Fixes #869 🦕