fix(a2asrv): skip TaskID mismatch check when message has no task reference (fixes #350) - #359
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the task ID validation in a2asrv/agentexec.go to allow an empty message.TaskID. The review feedback suggests improving the error handling for mismatched task IDs by wrapping the error with a2a.ErrInvalidParams and including the mismatched IDs in the error message, which prevents misclassifying client-side errors as internal server bugs.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if message.TaskID != "" && message.TaskID != tid { | ||
| return nil, fmt.Errorf("bug: message task id different from executor task id") | ||
| } |
There was a problem hiding this comment.
To improve debugging and align with the error handling pattern used for context ID mismatches (on line 203), consider wrapping this error with a2a.ErrInvalidParams and including the mismatched IDs in the error message. This avoids misclassifying a client-side parameter mismatch as an internal server bug.
| if message.TaskID != "" && message.TaskID != tid { | |
| return nil, fmt.Errorf("bug: message task id different from executor task id") | |
| } | |
| if message.TaskID != "" && message.TaskID != tid { | |
| return nil, fmt.Errorf("message task id %q different from executor task id %q: %w", message.TaskID, tid, a2a.ErrInvalidParams) | |
| } |
|
Thanks for tracking this down and preparing the fix, do you mind adding a test? |
…rence (fixes a2aproject#350) When loadExecutionContext receives a message without a TaskID (e.g. on retry in cluster mode), the condition message.TaskID != tid falsely rejects it because the empty-string TaskID cannot match any real tid. Add an empty-string guard so messages with no task reference are accepted. Add regression test that constructs a factory with taskRetrySupported=true and verifies loadExecutionContext accepts a message with empty TaskID when the task exists in the store.
433401e to
0a76c0f
Compare
|
Added regression test |
🤖 I have created a release *beep* *boop* --- ## [2.4.0](v2.3.1...v2.4.0) (2026-07-28) ### Features * provide generic pull event queue ([#354](#354)) ([6a48d4b](6a48d4b)) * **push:** push sender SSRF protection enabled by default (fixes [#373](#373)) ([#374](#374)) ([0a4f17a](0a4f17a)) ### Bug Fixes * **a2acompat/a2av0:** implement A2A v0.3 REST wire format ([#371](#371)) ([1ca80f9](1ca80f9)), closes [#370](#370) * **a2asrv:** skip TaskID mismatch check when message has no task reference (fixes [#350](#350)) ([#359](#359)) ([8c0dd99](8c0dd99)) * allow empty request bodies and enable GET method for task subscriptions ([#381](#381)) ([8363365](8363365)), closes [#380](#380) * **cli:** emit non-null required Agent Card list fields in synthesized card (fixes [#369](#369)) ([#372](#372)) ([0640869](0640869)) * **eventqueue:** add JSON marshal/unmarshal to Message so events survive roundtrip (fixes [#349](#349)) ([#360](#360)) ([d52d5a1](d52d5a1)) * itk grpc compatibility for v0.3 SDKs ([#367](#367)) ([11340a7](11340a7)) * ListTaskshistoryLength missing ([8b91364](8b91364)) * ListTaskshistoryLength missing (issue [#355](#355)) ([#361](#361)) ([8b91364](8b91364)) * **taskstore,push:** enforce cross-tenant authorization on Get and push config stores ([#357](#357)) ([a9f9c64](a9f9c64)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: Serob Nahapetyan <serob@google.com>
What
Fix
loadExecutionContextto not reject messages that have noTaskIDset.Why
In cluster mode with retries enabled, the function reaches the
TaskIDmismatch check at line 195 even for messages that never referenced any task (message.TaskID == ""). Since an empty string can never equal a realtid, this condition was always true, causing a false"bug: message task id different from executor task id"error.Fix
Add an empty-string guard:
Messages with no task reference are now accepted (they will be handled by the subsequent context creation / history replay logic), while messages that DO reference a task but the wrong one are still correctly rejected.
Closes #350