fix: rename Blocking to ReturnImmediately and implement return-immediately behavior#339
fix: rename Blocking to ReturnImmediately and implement return-immediately behavior#339rokonec wants to merge 2 commits intoa2aproject:mainfrom
Conversation
…ately behavior - rename SendMessageConfiguration.Blocking to ReturnImmediately matching A2A v1 spec - add MaterializeReturnImmediatelyResponseAsync with background event drain - track background tasks for cancellation via tasks/cancel and graceful shutdown - add 6 server unit tests covering return-immediately, cancel, and dispose 🐛 - Generated by Copilot
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 introduces a significant enhancement to the A2A server's message handling by implementing a "return-immediately" execution mode, which aligns the SDK with the A2A v1 specification for non-blocking operations. This allows clients to receive an initial task response quickly while the server efficiently manages and completes the remaining work in the background. The changes also include robust mechanisms for managing and gracefully shutting down these background tasks, alongside improvements to agent card discovery and comprehensive updates to samples and tests to demonstrate and validate the new behavior. Highlights
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. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a "return-immediately" (non-blocking) communication pattern for agents, allowing clients to receive an immediate response with an in-progress task while the agent continues processing in the background. The changes involve updating the SendMessageConfiguration to use ReturnImmediately, adding a new sample demonstration, and significantly refactoring the A2AServer to manage background tasks and their cancellation. The A2AServer now implements IAsyncDisposable and uses ConcurrentDictionary to track background operations. The review comments highlight several critical issues related to the use of ConcurrentDictionary in A2AServer, specifically that using indexers to add items to _backgroundCancellations and _backgroundTasks can lead to overwriting existing entries, causing resource leaks and making background tasks un-cancellable. It is recommended to use TryAdd for safer and more explicit handling. Additionally, the iteration and removal of items from _backgroundCancellations during disposal is identified as unreliable due to potential issues with ConcurrentDictionary enumeration while modifying the collection; a safer pattern involving taking a snapshot of keys before iteration is suggested.
| } | ||
| }, CancellationToken.None); | ||
|
|
||
| _backgroundTasks[context.TaskId] = drainTask; |
There was a problem hiding this comment.
Similar to the issue with _backgroundCancellations, using an indexer on _backgroundTasks can overwrite an existing entry. This could lead to an old background task being orphaned if a new one is added for the same task ID before the old one completes. Using TryAdd would be safer and would make the intention of adding a new, unique background task explicit. If TryAdd fails, it would indicate a logic error that should be investigated, as there should not be an existing background task if the CancellationTokenSource was successfully added.
There was a problem hiding this comment.
Fixed. The drain task is now tracked in _backgroundTasks and shares the same CTS token as the original handler (via CTS reuse for continuations). The _backgroundTasks overwrite for the same taskId is acceptable here — the prior drain is either still running (sharing the same CTS, so cancel reaches both) or already completed (removed itself in finally).
ba13faa to
a1f0c1a
Compare
- register /.well-known/agent-card.json at root in MapA2A() framework method - remove speccompliance-only special case from sample Program.cs 🔧 - Generated by Copilot
a1f0c1a to
b5c1f98
Compare
Summary
Addresses two issues from #329:
SendMessageConfiguration.Blocking→ReturnImmediatelyto match the A2A v1 spec wire format"returnImmediately"(proto fieldreturn_immediately).A2AServer.SendMessageAsync— whenReturnImmediately = true, the server returns the first Task event immediately and continues processing in the background.Also fixes agent card discovery and sample bugs discovered during validation.
Changes
Core SDK
SendMessageConfiguration.ReturnImmediately— Renamed fromBlockingwith inverted semantics. Defaultfalse= blocking (wait for completion), matching spec Section 3.2.2.A2AServer.SendMessageAsync— ChecksReturnImmediately, returns first Task event immediately via newMaterializeReturnImmediatelyResponseAsyncmethod. Background drain continues applying events to the task store.A2AServer : IAsyncDisposable— Background tasks tracked inConcurrentDictionaryfor cancellation viatasks/canceland graceful shutdown viaDisposeAsync().RequestContext.Configuration— Propagated fromSendMessageRequestso agents can inspectReturnImmediatelyand adapt behavior.Log.BackgroundEventProcessingFailed— Source-generated log method (event ID 3) for background drain errors.Agent Card Discovery
MapA2A()— Now registers/.well-known/agent-card.jsonat both the domain root (spec Section 8.2) and the path prefix (backward compat).Samples
EchoAgentWithTasks— Simulates slow work whenReturnImmediately = true; addedWorkingcase fortask-target-statemetadata.TaskBasedCommunicationSample— AddedDemoReturnImmediatelyAsyncshowing send → immediate response → poll for completion.Program.cs—baseUrlderived from--urlsarg so agent cards match actual listening port. Removed speccompliance-onlyMapWellKnownAgentCardspecial case.task-target-stateserialized as"Working"string instead of"TASK_STATE_WORKING"enum value.Tests (6 new)
returnImmediatelyCancelTaskAsynccancels background return-immediately handlerDisposeAsynccancels background work on shutdownValidation
Closes #329