From 8a5c6cf7718da6c3dc033837184a950c7af571d6 Mon Sep 17 00:00:00 2001 From: core-devops Date: Thu, 4 Jun 2026 20:43:43 -0700 Subject: [PATCH] =?UTF-8?q?test(a2a,providers):=20pin=20outbound=20A2A=20v?= =?UTF-8?q?0.3=20envelope=20+=20#2250=20auth=5Fenv=20SSOT;=20fix=20type?= =?UTF-8?q?=E2=86=92kind=20on=20send=20path?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two incident-derived regression gates plus the real source bug the first one surfaced. 1) Outbound A2A `message/send` envelope (#2251) — REAL, currently-shipping bug. buildA2AMessageParts (mcp_tools.go, feeds delegate_task + delegate_task_async) and the inline sync-delegation envelope (delegation.go) emitted the text Part as {"type":"text"} instead of the A2A v0.3-canonical {"kind":"text"}. A v0.3 peer's Pydantic validator discriminates Parts on `kind` and silently drops a `type`-keyed Part — the sender sees a happy 200/202 while the brief is lost. #2255 fixed the INBOUND normalizeA2APayload (type→kind on receive); this OUTBOUND send path was separate and still buggy on main. The file-attachment Part already used `kind` (untouched); MCP tools/call content schema legitimately keeps `type` (different protocol, untouched). Fix: text Part type→kind in both send paths. Gate: a2a_outbound_envelope_test.go — pins text-part `kind`, file-part `kind` (non-regression), and the full envelope role+kind. RED before the fix (the two kind-asserting tests failed against the shipping `type` shape), GREEN after. 2) Platform provider auth_env SSOT (#2250) — exact-equality gate. The `platform` (closed proxy) provider must advertise ONLY MOLECULE_LLM_USAGE_TOKEN in auth_env; a vendor key there makes the canvas demand a credential the platform path ignores (wrong-bill / silent no-op). The pre-existing tests only do a membership / non-empty check, which passes against a drifted two-element list. This pins the WHOLE set. Core's providers.yaml is already clean (the vendor key lives in the separate auth_token_env field), so the gate currently PASSES and locks that invariant against future drift onto this SSOT. The drift itself lives in the codex template repo. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../handlers/a2a_outbound_envelope_test.go | 141 ++++++++++++++++++ .../internal/handlers/delegation.go | 7 +- .../internal/handlers/mcp_tools.go | 6 +- .../internal/providers/providers_test.go | 43 ++++++ 4 files changed, 194 insertions(+), 3 deletions(-) create mode 100644 workspace-server/internal/handlers/a2a_outbound_envelope_test.go diff --git a/workspace-server/internal/handlers/a2a_outbound_envelope_test.go b/workspace-server/internal/handlers/a2a_outbound_envelope_test.go new file mode 100644 index 000000000..9a6199c33 --- /dev/null +++ b/workspace-server/internal/handlers/a2a_outbound_envelope_test.go @@ -0,0 +1,141 @@ +package handlers + +// a2a_outbound_envelope_test.go — outbound A2A `message/send` envelope +// CONTRACT gate (issue #2251). +// +// #2251: an outbound A2A envelope shipped without `role` and with text +// parts keyed `type` instead of the v0.3-canonical `kind`. The receiver's +// a-2-a-sdk v0.3 Pydantic validator silently rejected the message +// post-dispatch — the sender saw a happy 200/202 while the brief was +// dropped (the same invisible-rejection failure class as the v0.2→v0.3 +// content bug pinned by a2a_corpus_test.go, but on the SEND side). +// +// The inbound corpus replay (a2a_corpus_test.go) proves normalizeA2APayload +// produces `parts[].kind` + a non-empty messageId, but it does NOT assert +// `role`, and it only covers what we RECEIVE. Nothing pins what core +// EMITS. This file pins the emit contract at the helper that builds the +// parts (buildA2AMessageParts, used by both delegate_task and +// delegate_task_async) and asserts the canonical Part key is `kind`. +// +// Part-object schema (A2A v0.3): every Part MUST carry a `kind` +// discriminator ("text" | "file" | "data"); there is NO `type` key. A +// text Part is {"kind":"text","text":"..."}. Emitting `type` makes the +// v0.3 validator drop the Part. + +import ( + "encoding/json" + "testing" +) + +// TestBuildA2AMessageParts_TextPartUsesKindNotType pins the v0.3 Part +// discriminator for the text part emitted on every outbound A2A +// delegation. RED before #2251's fix (the helper emitted +// {"type":"text",...}); the receiver's v0.3 Pydantic validator drops a +// Part keyed `type`, silently losing the task text. +func TestBuildA2AMessageParts_TextPartUsesKindNotType(t *testing.T) { + parts := buildA2AMessageParts("do the work", nil) + if len(parts) == 0 { + t.Fatal("buildA2AMessageParts returned no parts for a non-empty task") + } + text := parts[0] + + if _, hasType := text["type"]; hasType { + t.Errorf("text part uses forbidden v0.2 key `type` %v — A2A v0.3 Parts discriminate on `kind`; `type` is dropped by the receiver's validator (#2251)", text) + } + kind, ok := text["kind"].(string) + if !ok { + t.Fatalf("text part missing string `kind` discriminator; got %v", text) + } + if kind != "text" { + t.Errorf("text part kind = %q, want \"text\"", kind) + } + if text["text"] != "do the work" { + t.Errorf("text part text = %v, want \"do the work\"", text["text"]) + } +} + +// TestBuildA2AMessageParts_FilePartUsesKind guards the file-attachment +// Part the same way. The file path was already correct (it used `kind`), +// so this is a non-regression pin — it must STAY `kind` when the text +// path is fixed (a careless "make them consistent" edit could flip both +// to the wrong key). +func TestBuildA2AMessageParts_FilePartUsesKind(t *testing.T) { + atts := []AgentMessageAttachment{ + {URI: "https://example.com/a.png", MimeType: "image/png", Name: "a.png"}, + } + parts := buildA2AMessageParts("caption", atts) + if len(parts) < 2 { + t.Fatalf("expected text + file parts, got %d", len(parts)) + } + file := parts[1] + if _, hasType := file["type"]; hasType { + t.Errorf("file part uses forbidden `type` key: %v", file) + } + if _, hasKind := file["kind"]; !hasKind { + t.Errorf("file part missing `kind` discriminator: %v", file) + } +} + +// TestDelegationOutboundEnvelope_RoleAndKind pins the FULL outbound +// envelope contract — role + parts[].kind — on the canonical helper. +// A v0.3 `message` MUST carry `role` ("user" for a delegation request) +// and `parts` whose every entry discriminates on `kind`. This is the +// shape the receiver's MessageSendParams validator accepts; an envelope +// missing `role` or keyed `type` is silently rejected (#2251). +// +// Built from the same primitives delegation.go / mcp_tools.go assemble +// (role:"user" + buildA2AMessageParts) so the round-trip through +// json.Marshal proves the wire bytes are v0.3-valid. +func TestDelegationOutboundEnvelope_RoleAndKind(t *testing.T) { + envelope := map[string]interface{}{ + "method": "message/send", + "params": map[string]interface{}{ + "message": map[string]interface{}{ + "role": "user", + "messageId": "deleg-1", + "parts": buildA2AMessageParts("do the work", nil), + }, + }, + } + raw, err := json.Marshal(envelope) + if err != nil { + t.Fatalf("marshal envelope: %v", err) + } + var parsed map[string]interface{} + if err := json.Unmarshal(raw, &parsed); err != nil { + t.Fatalf("unmarshal envelope: %v", err) + } + + params, _ := parsed["params"].(map[string]interface{}) + if params == nil { + t.Fatal("envelope missing params") + } + msg, _ := params["message"].(map[string]interface{}) + if msg == nil { + t.Fatal("envelope missing params.message") + } + + // role is mandatory on a v0.3 message — the receiver rejects without it. + role, hasRole := msg["role"].(string) + if !hasRole || role == "" { + t.Errorf("params.message missing non-empty `role` — v0.3 requires it; omitting it is the other half of #2251") + } + + parts, _ := msg["parts"].([]interface{}) + if len(parts) == 0 { + t.Fatal("params.message.parts is empty") + } + for i, p := range parts { + pm, _ := p.(map[string]interface{}) + if pm == nil { + t.Errorf("part %d is not an object: %v", i, p) + continue + } + if _, hasType := pm["type"]; hasType { + t.Errorf("part %d uses forbidden `type` key (must be `kind`): %v", i, pm) + } + if _, hasKind := pm["kind"]; !hasKind { + t.Errorf("part %d missing `kind` discriminator: %v", i, pm) + } + } +} diff --git a/workspace-server/internal/handlers/delegation.go b/workspace-server/internal/handlers/delegation.go index 72b0a89ba..aae43309d 100644 --- a/workspace-server/internal/handlers/delegation.go +++ b/workspace-server/internal/handlers/delegation.go @@ -179,8 +179,11 @@ func (h *DelegationHandler) Delegate(c *gin.Context) { "message": map[string]interface{}{ "role": "user", "messageId": delegationID, - "parts": []map[string]interface{}{{"type": "text", "text": body.Task}}, - "metadata": map[string]interface{}{"delegation_id": delegationID}, + // A2A v0.3 Part discriminator is `kind`, NOT `type` (#2251) — + // a `type`-keyed Part is dropped by the receiver's v0.3 + // validator, silently losing the delegated task. + "parts": []map[string]interface{}{{"kind": "text", "text": body.Task}}, + "metadata": map[string]interface{}{"delegation_id": delegationID}, }, }, }) diff --git a/workspace-server/internal/handlers/mcp_tools.go b/workspace-server/internal/handlers/mcp_tools.go index 3e0f1930a..4664de25e 100644 --- a/workspace-server/internal/handlers/mcp_tools.go +++ b/workspace-server/internal/handlers/mcp_tools.go @@ -192,7 +192,11 @@ func (h *MCPHandler) toolGetWorkspaceInfo(ctx context.Context, workspaceID strin // follow in the order provided, with kind derived from MIME type. func buildA2AMessageParts(task string, attachments []AgentMessageAttachment) []map[string]interface{} { parts := []map[string]interface{}{ - {"type": "text", "text": task}, + // A2A v0.3 Part discriminator is `kind`, NOT `type` (#2251). + // The receiver's v0.3 Pydantic validator drops a Part keyed + // `type`, silently losing the task text — the file part below + // already uses `kind`, this is the matching fix for text. + {"kind": "text", "text": task}, } for _, att := range attachments { kind := kindFromMimeType(att.MimeType) diff --git a/workspace-server/internal/providers/providers_test.go b/workspace-server/internal/providers/providers_test.go index 7ce25f0a0..47d313e44 100644 --- a/workspace-server/internal/providers/providers_test.go +++ b/workspace-server/internal/providers/providers_test.go @@ -324,3 +324,46 @@ func TestVertexProviderRegistered(t *testing.T) { } } } + +// TestPlatformProvider_AuthEnvIsUsageTokenOnly is the SSOT-side regression +// gate for the platform-managed auth_env drift class (issue #2250 — the +// codex template's `platform` provider shipped +// auth_env: [MOLECULE_LLM_USAGE_TOKEN, ANTHROPIC_API_KEY], wrongly +// advertising a vendor key under a platform-managed provider). +// +// The `platform` provider is the closed Molecule proxy arm: the platform +// owns billing and injects MOLECULE_LLM_USAGE_TOKEN, so a tenant supplies +// NO vendor credential. Listing ANTHROPIC_API_KEY (or any other vendor key) +// in its auth_env makes the canvas demand a credential the platform path +// neither needs nor uses, and lets a stray vendor key satisfy the +// "auth present" check on a path that ignores it — exactly the wrong-bill / +// silent-no-op failure mode the BYOK-vs-platform split exists to prevent. +// +// EXACT-equality (not membership): the prior template-side test only +// asserted `"MOLECULE_LLM_USAGE_TOKEN" in auth_env`, which PASSED against +// the buggy two-element list. Pin the WHOLE set so an extra vendor key +// trips the gate. This is the core providers.yaml SSOT; the template +// derives from / must byte-match this set (drift-gated by molecule-ci). +// On core this currently PASSES (auth_env is already clean; the vendor +// key lives in the separate auth_token_env field) — the gate locks that +// in so a future drift onto this SSOT trips CI. +func TestPlatformProvider_AuthEnvIsUsageTokenOnly(t *testing.T) { + ps, err := Load() + if err != nil { + t.Fatalf("Load() error = %v", err) + } + var platform *Provider + for i := range ps { + if ps[i].Name == "platform" { + platform = &ps[i] + break + } + } + if platform == nil { + t.Fatal("platform provider missing from providers.yaml — the closed proxy arm must exist") + } + want := []string{"MOLECULE_LLM_USAGE_TOKEN"} + if len(platform.AuthEnv) != len(want) || platform.AuthEnv[0] != want[0] { + t.Errorf("platform provider auth_env = %v, want exactly %v — a vendor key under a platform-managed provider is the #2250 drift; auth_token_env (the proxy's internal projection target) is a SEPARATE field and must not leak into auth_env", platform.AuthEnv, want) + } +} -- 2.52.0