fix(a2a): v0.2 → v0.3 compat shim at proxy edge (#2345)
Closes #2345. ## Symptom Design Director silently dropped A2A briefs whose sender used the v0.2 message format (`params.message.content` string) instead of v0.3 (`params.message.parts` part-list). The downstream a2a-sdk's v0.3 Pydantic validator rejected with "params.message.parts — Field required" but the rejection only landed in tenant-side logs; the sender saw HTTP 200/202 and assumed delivery. UX Researcher therefore never received the kickoff. Multi-agent pipeline silently idle. ## Fix Convert at the proxy edge in normalizeA2APayload. Two cases handled, one explicitly rejected: v0.2 string content → wrap as [{kind: text, text: <content>}] (the canonical v0.2 case from the dogfooding report) v0.2 list content → preserve list as parts (some older clients put a list under `content`; treat as "client meant parts, used wrong field name") v0.3 parts present → no-op (hot path for normal traffic) Neither present → return HTTP 400 with structured JSON-RPC error pointing at the missing field Why at the proxy edge: every workspace gets the compat for free without each one bumping a2a-sdk separately. The SDK's own compat adapter is strict about `parts` and rejects v0.2 senders. Why reject loud on missing-both: pre-fix the SDK's Pydantic rejection was post-handler-dispatch and invisible to the original sender. Now misshapen payloads return a structured 400 to the actual caller — kills the entire silent-drop class for this payload-shape category. ## Tests 7 new cases on normalizeA2APayload (#2345) + 1 fixture update on the existing _MissingMethodReturnsEmpty test: TestNormalizeA2APayload_ConvertsV02StringContentToParts TestNormalizeA2APayload_ConvertsV02ListContentToParts TestNormalizeA2APayload_PreservesV03Parts (hot path) TestNormalizeA2APayload_RejectsMessageWithNeitherContentNorParts TestNormalizeA2APayload_RejectsContentWithUnsupportedType TestNormalizeA2APayload_NoMessageNoCheck (e.g. tasks/list bypasses) All 11 normalizeA2APayload tests pass + full handler suite (no regressions). ## Refs Hard-gates discussion: this is exactly the class of failure (silent-drop on schema mismatch) that #2342 (continuous synthetic E2E) would catch automatically. Tier 2 RFC item from #2345 (caller gets structured JSON-RPC error on parse failure) is delivered above via the loud-reject path.
This commit is contained in:
Hongming Wang
parent
21ed74c76a
commit
140fc5fb10
@ -486,11 +486,54 @@ func normalizeA2APayload(body []byte) ([]byte, string, *proxyA2AError) {
|
||||
}
|
||||
|
||||
// Ensure params.message.messageId exists (required by a2a-sdk)
|
||||
// AND v0.2→v0.3 compat (#2345): when sender supplies
|
||||
// params.message.content (v0.2) instead of params.message.parts
|
||||
// (v0.3), wrap the content as a single text Part so the downstream
|
||||
// a2a-sdk's v0.3 Pydantic validator accepts the message.
|
||||
//
|
||||
// Pre-fix: Design Director silently dropped briefs whose sender
|
||||
// used v0.2 shape — Pydantic rejected at parse time, the rejection
|
||||
// went only to logs, and the sender saw a happy 200/202.
|
||||
//
|
||||
// Reject loud (HTTP 400) when neither content nor parts is present;
|
||||
// previously the SDK's own rejection happened post-handler-dispatch
|
||||
// and was invisible to the original sender.
|
||||
if params, ok := payload["params"].(map[string]interface{}); ok {
|
||||
if msg, ok := params["message"].(map[string]interface{}); ok {
|
||||
if _, hasID := msg["messageId"]; !hasID {
|
||||
msg["messageId"] = uuid.New().String()
|
||||
}
|
||||
_, hasParts := msg["parts"]
|
||||
rawContent, hasContent := msg["content"]
|
||||
if !hasParts {
|
||||
if hasContent {
|
||||
switch v := rawContent.(type) {
|
||||
case string:
|
||||
msg["parts"] = []interface{}{
|
||||
map[string]interface{}{"kind": "text", "text": v},
|
||||
}
|
||||
case []interface{}:
|
||||
msg["parts"] = v
|
||||
default:
|
||||
return nil, "", &proxyA2AError{
|
||||
Status: http.StatusBadRequest,
|
||||
Response: gin.H{
|
||||
"error": "invalid params.message.content type",
|
||||
"hint": "content must be a string (v0.2 compat) or omitted in favour of parts (v0.3)",
|
||||
},
|
||||
}
|
||||
}
|
||||
delete(msg, "content")
|
||||
} else {
|
||||
return nil, "", &proxyA2AError{
|
||||
Status: http.StatusBadRequest,
|
||||
Response: gin.H{
|
||||
"error": "params.message must contain either 'parts' (v0.3) or 'content' (v0.2 compat)",
|
||||
"hint": "v0.3 example: {\"parts\":[{\"kind\":\"text\",\"text\":\"...\"}]}",
|
||||
},
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -11,6 +11,7 @@ import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@ -1137,7 +1138,10 @@ func TestNormalizeA2APayload_PreservesExistingMessageId(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestNormalizeA2APayload_MissingMethodReturnsEmpty(t *testing.T) {
|
||||
raw := []byte(`{"params":{"message":{"role":"user"}}}`)
|
||||
// Method extraction returns empty string when method is absent,
|
||||
// regardless of message validity. Include parts: [] so the v0.2→v0.3
|
||||
// compat check (#2345) doesn't reject before method extraction.
|
||||
raw := []byte(`{"params":{"message":{"role":"user","parts":[]}}}`)
|
||||
_, method, perr := normalizeA2APayload(raw)
|
||||
if perr != nil {
|
||||
t.Fatalf("unexpected error: %+v", perr)
|
||||
@ -1147,6 +1151,102 @@ func TestNormalizeA2APayload_MissingMethodReturnsEmpty(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// --- v0.2 → v0.3 compat shim (#2345) ---
|
||||
|
||||
func TestNormalizeA2APayload_ConvertsV02StringContentToParts(t *testing.T) {
|
||||
raw := []byte(`{"method":"message/send","params":{"message":{"role":"user","content":"hello world"}}}`)
|
||||
out, _, perr := normalizeA2APayload(raw)
|
||||
if perr != nil {
|
||||
t.Fatalf("unexpected error: %+v", perr)
|
||||
}
|
||||
var parsed map[string]interface{}
|
||||
if err := json.Unmarshal(out, &parsed); err != nil {
|
||||
t.Fatalf("output not valid JSON: %v", err)
|
||||
}
|
||||
msg := parsed["params"].(map[string]interface{})["message"].(map[string]interface{})
|
||||
if _, stillHasContent := msg["content"]; stillHasContent {
|
||||
t.Error("v0.2 'content' field should be removed after conversion")
|
||||
}
|
||||
parts, ok := msg["parts"].([]interface{})
|
||||
if !ok || len(parts) != 1 {
|
||||
t.Fatalf("expected 1 part, got %v", msg["parts"])
|
||||
}
|
||||
part := parts[0].(map[string]interface{})
|
||||
if part["kind"] != "text" || part["text"] != "hello world" {
|
||||
t.Errorf("expected {kind:text, text:'hello world'}, got %v", part)
|
||||
}
|
||||
}
|
||||
|
||||
func TestNormalizeA2APayload_ConvertsV02ListContentToParts(t *testing.T) {
|
||||
raw := []byte(`{"method":"message/send","params":{"message":{"role":"user","content":[{"kind":"text","text":"hi"}]}}}`)
|
||||
out, _, perr := normalizeA2APayload(raw)
|
||||
if perr != nil {
|
||||
t.Fatalf("unexpected error: %+v", perr)
|
||||
}
|
||||
var parsed map[string]interface{}
|
||||
_ = json.Unmarshal(out, &parsed)
|
||||
msg := parsed["params"].(map[string]interface{})["message"].(map[string]interface{})
|
||||
parts, ok := msg["parts"].([]interface{})
|
||||
if !ok || len(parts) != 1 {
|
||||
t.Fatalf("expected list preserved as parts, got %v", msg["parts"])
|
||||
}
|
||||
}
|
||||
|
||||
func TestNormalizeA2APayload_PreservesV03Parts(t *testing.T) {
|
||||
raw := []byte(`{"method":"message/send","params":{"message":{"role":"user","parts":[{"kind":"text","text":"hi"}]}}}`)
|
||||
out, _, perr := normalizeA2APayload(raw)
|
||||
if perr != nil {
|
||||
t.Fatalf("unexpected error: %+v", perr)
|
||||
}
|
||||
var parsed map[string]interface{}
|
||||
_ = json.Unmarshal(out, &parsed)
|
||||
msg := parsed["params"].(map[string]interface{})["message"].(map[string]interface{})
|
||||
if _, hasContent := msg["content"]; hasContent {
|
||||
t.Error("did not expect content field in v0.3-shaped payload output")
|
||||
}
|
||||
parts := msg["parts"].([]interface{})
|
||||
if len(parts) != 1 {
|
||||
t.Errorf("expected 1 part preserved, got %d", len(parts))
|
||||
}
|
||||
}
|
||||
|
||||
func TestNormalizeA2APayload_RejectsMessageWithNeitherContentNorParts(t *testing.T) {
|
||||
raw := []byte(`{"method":"message/send","params":{"message":{"role":"user","metadata":{}}}}`)
|
||||
_, _, perr := normalizeA2APayload(raw)
|
||||
if perr == nil {
|
||||
t.Fatal("expected error for message with neither content nor parts")
|
||||
}
|
||||
if perr.Status != http.StatusBadRequest {
|
||||
t.Errorf("expected 400, got %d", perr.Status)
|
||||
}
|
||||
errMsg, _ := perr.Response["error"].(string)
|
||||
if !strings.Contains(errMsg, "parts") || !strings.Contains(errMsg, "content") {
|
||||
t.Errorf("error message should mention both 'parts' and 'content', got: %q", errMsg)
|
||||
}
|
||||
}
|
||||
|
||||
func TestNormalizeA2APayload_RejectsContentWithUnsupportedType(t *testing.T) {
|
||||
raw := []byte(`{"method":"message/send","params":{"message":{"role":"user","content":42}}}`)
|
||||
_, _, perr := normalizeA2APayload(raw)
|
||||
if perr == nil {
|
||||
t.Fatal("expected error for non-string non-list content")
|
||||
}
|
||||
if perr.Status != http.StatusBadRequest {
|
||||
t.Errorf("expected 400, got %d", perr.Status)
|
||||
}
|
||||
}
|
||||
|
||||
func TestNormalizeA2APayload_NoMessageNoCheck(t *testing.T) {
|
||||
raw := []byte(`{"method":"tasks/list","params":{}}`)
|
||||
_, method, perr := normalizeA2APayload(raw)
|
||||
if perr != nil {
|
||||
t.Fatalf("unexpected error on params-message-absent payload: %+v", perr)
|
||||
}
|
||||
if method != "tasks/list" {
|
||||
t.Errorf("expected method=tasks/list, got %q", method)
|
||||
}
|
||||
}
|
||||
|
||||
// --- resolveAgentURL direct unit tests ---
|
||||
|
||||
func TestResolveAgentURL_CacheHit(t *testing.T) {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user