From b2dabe2ed8fe8df18f78cab3656b7ee630e1d1bf Mon Sep 17 00:00:00 2001 From: Molecule AI Core-BE Date: Tue, 12 May 2026 07:58:24 +0000 Subject: [PATCH 01/11] =?UTF-8?q?test(handlers/a2a=5Fproxy=5Fhelpers):=20a?= =?UTF-8?q?dd=20a2a=5Fproxy=5Fhelpers=5Ftest.go=20=E2=80=94=2020=20cases?= =?UTF-8?q?=20for=20pure=20helpers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Covers nilIfEmpty, extractToolTrace, readUsageMap, parseUsageFromA2AResponse. extractToolTrace: 8 cases including empty/invalid JSON, missing result/metadata/ tool_trace keys, null value (mc#669 regression), empty array, valid non-empty. readUsageMap: 5 cases covering no key, invalid usage JSON, zero/non-zero tokens. parseUsageFromA2AResponse: 8 cases covering empty, invalid JSON, result.usage priority over top-level, top-level fallback, zero values, missing fields. extractToolTrace null-value case documents the mc#669 json.RawMessage bug (len(nil) panic on JSON null); TestExtractToolTrace_NullValue asserts the correct post-fix behavior (nil return). --- .../handlers/a2a_proxy_helpers_test.go | 243 ++++++++++++++++++ 1 file changed, 243 insertions(+) create mode 100644 workspace-server/internal/handlers/a2a_proxy_helpers_test.go diff --git a/workspace-server/internal/handlers/a2a_proxy_helpers_test.go b/workspace-server/internal/handlers/a2a_proxy_helpers_test.go new file mode 100644 index 00000000..48097e62 --- /dev/null +++ b/workspace-server/internal/handlers/a2a_proxy_helpers_test.go @@ -0,0 +1,243 @@ +package handlers + +import ( + "encoding/json" + "testing" +) + +// ───────────────────────────────────────────────────────────────────────────── +// nilIfEmpty tests +// ───────────────────────────────────────────────────────────────────────────── + +func TestNilIfEmpty_EmptyString(t *testing.T) { + got := nilIfEmpty("") + if got != nil { + t.Errorf("empty string: got %p, want nil", got) + } +} + +func TestNilIfEmpty_NonEmptyString(t *testing.T) { + s := "hello" + got := nilIfEmpty(s) + if got == nil { + t.Fatal("non-empty string: got nil, want pointer") + } + if *got != "hello" { + t.Errorf("non-empty string: got %q, want %q", *got, "hello") + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// extractToolTrace tests +// ───────────────────────────────────────────────────────────────────────────── + +func TestExtractToolTrace_EmptyBody(t *testing.T) { + got := extractToolTrace(nil) + if got != nil { + t.Errorf("nil body: got %v, want nil", got) + } + got = extractToolTrace([]byte{}) + if got != nil { + t.Errorf("empty body: got %v, want nil", got) + } +} + +func TestExtractToolTrace_InvalidJSON(t *testing.T) { + got := extractToolTrace([]byte("not json")) + if got != nil { + t.Errorf("invalid JSON: got %v, want nil", got) + } +} + +func TestExtractToolTrace_NoResultKey(t *testing.T) { + got := extractToolTrace([]byte(`{"error": "oops"}`)) + if got != nil { + t.Errorf("no result key: got %v, want nil", got) + } +} + +func TestExtractToolTrace_NoMetadataKey(t *testing.T) { + got := extractToolTrace([]byte(`{"result": {"data": {}}}`)) + if got != nil { + t.Errorf("no metadata key: got %v, want nil", got) + } +} + +func TestExtractToolTrace_NoToolTraceKey(t *testing.T) { + got := extractToolTrace([]byte(`{"result": {"metadata": {}}}`)) + if got != nil { + t.Errorf("no tool_trace key: got %v, want nil", got) + } +} + +// extractToolTrace calls json.Unmarshal, which sets a RawMessage to nil when +// unmarshaling a JSON null value. The fix for mc#669 changes len(trace)==0 +// to string(trace)=="[]" to avoid len(nil) panicking on null. +func TestExtractToolTrace_NullValue(t *testing.T) { + // JSON null in tool_trace → RawMessage becomes nil → len would panic. + // The fix checks string(trace)=="[]" which is safe on nil (returns false). + body := []byte(`{"result": {"metadata": {"tool_trace": null}}}`) + got := extractToolTrace(body) + if got != nil { + t.Errorf("null tool_trace: got %v, want nil", got) + } +} + +// "[]" unmarshaled into RawMessage is []byte("[]") — not nil, len=2. +// The fix returns nil for [] so empty tool_trace arrays don't surface as traces. +func TestExtractToolTrace_EmptyArray(t *testing.T) { + body := []byte(`{"result": {"metadata": {"tool_trace": []}}}`) + got := extractToolTrace(body) + if got != nil { + t.Errorf("empty array tool_trace: got %v, want nil", got) + } +} + +func TestExtractToolTrace_ValidNonEmpty(t *testing.T) { + trace := []byte(`[{"name": "search", "result": "done"}]`) + body, _ := json.Marshal(map[string]interface{}{ + "result": map[string]interface{}{ + "metadata": map[string]interface{}{ + "tool_trace": json.RawMessage(trace), + }, + }, + }) + got := extractToolTrace(body) + if got == nil { + t.Fatal("valid non-empty trace: got nil, want the trace") + } + if string(got) != string(trace) { + t.Errorf("valid trace: got %s, want %s", got, trace) + } +} + +// Document that the CURRENT code (len check) panics on null tool_trace. +// This test exists to signal when PR #669's fix lands: after the fix, +// the defer-recover will NOT trigger (panic goes away) and the +// post-recover assertion runs. While unfixed: the panic fires and + +// ───────────────────────────────────────────────────────────────────────────── +// readUsageMap tests +// ───────────────────────────────────────────────────────────────────────────── + +func TestReadUsageMap_NoUsageKey(t *testing.T) { + m := map[string]json.RawMessage{} + in, out, ok := readUsageMap(m) + if ok { + t.Error("no usage key: ok should be false") + } +} + +func TestReadUsageMap_InvalidUsageJSON(t *testing.T) { + m := map[string]json.RawMessage{"usage": json.RawMessage(`"not an object"`)} + in, out, ok := readUsageMap(m) + if ok { + t.Error("invalid usage JSON: ok should be false") + } +} + +func TestReadUsageMap_ZeroUsage(t *testing.T) { + m := map[string]json.RawMessage{"usage": json.RawMessage(`{"input_tokens": 0, "output_tokens": 0}`)} + in, out, ok := readUsageMap(m) + if ok { + t.Error("zero usage: ok should be false") + } +} + +func TestReadUsageMap_InputOnly(t *testing.T) { + m := map[string]json.RawMessage{"usage": json.RawMessage(`{"input_tokens": 100, "output_tokens": 0}`)} + in, out, ok := readUsageMap(m) + if !ok { + t.Fatal("input-only usage: ok should be true") + } + if in != 100 { + t.Errorf("input tokens: got %d, want 100", in) + } + if out != 0 { + t.Errorf("output tokens: got %d, want 0", out) + } +} + +func TestReadUsageMap_BothTokens(t *testing.T) { + m := map[string]json.RawMessage{"usage": json.RawMessage(`{"input_tokens": 500, "output_tokens": 200}`)} + in, out, ok := readUsageMap(m) + if !ok { + t.Fatal("both tokens: ok should be true") + } + if in != 500 || out != 200 { + t.Errorf("tokens: got (%d, %d), want (500, 200)", in, out) + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// parseUsageFromA2AResponse tests +// ───────────────────────────────────────────────────────────────────────────── + +func TestParseUsageFromA2AResponse_Empty(t *testing.T) { + in, out := parseUsageFromA2AResponse(nil) + if in != 0 || out != 0 { + t.Errorf("nil: got (%d, %d), want (0, 0)", in, out) + } + in, out = parseUsageFromA2AResponse([]byte{}) + if in != 0 || out != 0 { + t.Errorf("empty: got (%d, %d), want (0, 0)", in, out) + } +} + +func TestParseUsageFromA2AResponse_InvalidJSON(t *testing.T) { + in, out := parseUsageFromA2AResponse([]byte("not json")) + if in != 0 || out != 0 { + t.Errorf("invalid JSON: got (%d, %d), want (0, 0)", in, out) + } +} + +func TestParseUsageFromA2AResponse_NoResultNoUsage(t *testing.T) { + in, out := parseUsageFromA2AResponse([]byte(`{"id": 1}`)) + if in != 0 || out != 0 { + t.Errorf("no result/usage: got (%d, %d), want (0, 0)", in, out) + } +} + +func TestParseUsageFromA2AResponse_ResultUsage(t *testing.T) { + body := []byte(`{"result": {"usage": {"input_tokens": 42, "output_tokens": 7}}}`) + in, out := parseUsageFromA2AResponse(body) + if in != 42 || out != 7 { + t.Errorf("result usage: got (%d, %d), want (42, 7)", in, out) + } +} + +func TestParseUsageFromA2AResponse_ResultUsageWinsOverTopLevel(t *testing.T) { + // JSON-RPC result.usage takes precedence over top-level usage. + body := []byte(`{"result": {"usage": {"input_tokens": 42, "output_tokens": 7}}, "usage": {"input_tokens": 99, "output_tokens": 99}}`) + in, out := parseUsageFromA2AResponse(body) + if in != 42 || out != 7 { + t.Errorf("result usage should win: got (%d, %d), want (42, 7)", in, out) + } +} + +func TestParseUsageFromA2AResponse_TopLevelFallback(t *testing.T) { + // Direct (non-JSON-RPC) response: usage at top level. + body := []byte(`{"usage": {"input_tokens": 11, "output_tokens": 13}}`) + in, out := parseUsageFromA2AResponse(body) + if in != 11 || out != 13 { + t.Errorf("top-level usage: got (%d, %d), want (11, 13)", in, out) + } +} + +func TestParseUsageFromA2AResponse_ZeroValuesInResult(t *testing.T) { + // Zero usage in result.result.usage: returns (0, 0) — no panic. + body := []byte(`{"result": {"usage": {"input_tokens": 0, "output_tokens": 0}}}`) + in, out := parseUsageFromA2AResponse(body) + if in != 0 || out != 0 { + t.Errorf("zero usage: got (%d, %d), want (0, 0)", in, out) + } +} + +func TestParseUsageFromA2AResponse_MissingTokensInUsageObject(t *testing.T) { + // usage object exists but tokens are absent — returns (0, 0). + body := []byte(`{"result": {"usage": {"other_field": 5}}}`) + in, out := parseUsageFromA2AResponse(body) + if in != 0 || out != 0 { + t.Errorf("missing tokens: got (%d, %d), want (0, 0)", in, out) + } +} -- 2.45.2 From 1d392782830950228085bf307cef216e5f2fc22f Mon Sep 17 00:00:00 2001 From: core-devops Date: Tue, 12 May 2026 20:52:01 +0000 Subject: [PATCH 02/11] ci: rerun after mc#724 all-required fix lands -- 2.45.2 From 9a5226ee82eae7c683c1b45e155f66f07e0099f0 Mon Sep 17 00:00:00 2001 From: claude-ceo-assistant Date: Tue, 12 May 2026 21:17:12 +0000 Subject: [PATCH 03/11] ci: rerun after concurrency-block clear -- 2.45.2 From 8210e069a679bd5b9c6aa8d5ae75430e2ba8880e Mon Sep 17 00:00:00 2001 From: claude-ceo-assistant Date: Tue, 12 May 2026 21:25:51 +0000 Subject: [PATCH 04/11] ci: clean-slate rerun -- 2.45.2 From 4973d5ff196caeb93f0eec0c8ee48f6802c364eb Mon Sep 17 00:00:00 2001 From: claude-ceo-assistant Date: Tue, 12 May 2026 21:30:35 +0000 Subject: [PATCH 05/11] ci: post-restart rerun -- 2.45.2 From 97fffa0485f511e411107b2024e7c86ed420efc4 Mon Sep 17 00:00:00 2001 From: core-lead Date: Tue, 12 May 2026 21:35:21 +0000 Subject: [PATCH 06/11] ci: clean-slate rerun v2 -- 2.45.2 From d01148e78a373fd006e5627db39d6d1946ab7377 Mon Sep 17 00:00:00 2001 From: core-lead Date: Tue, 12 May 2026 21:44:47 +0000 Subject: [PATCH 07/11] ci: global-zombie-purge rerun -- 2.45.2 From 2672cdb2d199382da6e13b80262cf4243d52de37 Mon Sep 17 00:00:00 2001 From: core-lead Date: Tue, 12 May 2026 21:48:41 +0000 Subject: [PATCH 08/11] ci: post-full-purge rerun -- 2.45.2 From c9573815ef043c8deba4750c7edce75aa270fcb5 Mon Sep 17 00:00:00 2001 From: core-devops Date: Tue, 12 May 2026 22:07:29 +0000 Subject: [PATCH 09/11] ci: post-purge rerun -- 2.45.2 From 31d14a4cf6f71f0b02753ce92abbee16a9f6a075 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-Security Date: Tue, 12 May 2026 18:41:19 -0700 Subject: [PATCH 10/11] fix(test/handlers): use blank identifiers for unused vars in negative readUsageMap tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Go disallows declared-but-unused variables; in tests that check ok==false, in and out are irrelevant — replace with _. Co-Authored-By: claude-sonnet-4-6 --- .../internal/handlers/a2a_proxy_helpers_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/workspace-server/internal/handlers/a2a_proxy_helpers_test.go b/workspace-server/internal/handlers/a2a_proxy_helpers_test.go index 48097e62..273c6750 100644 --- a/workspace-server/internal/handlers/a2a_proxy_helpers_test.go +++ b/workspace-server/internal/handlers/a2a_proxy_helpers_test.go @@ -122,7 +122,7 @@ func TestExtractToolTrace_ValidNonEmpty(t *testing.T) { func TestReadUsageMap_NoUsageKey(t *testing.T) { m := map[string]json.RawMessage{} - in, out, ok := readUsageMap(m) + _, _, ok := readUsageMap(m) if ok { t.Error("no usage key: ok should be false") } @@ -130,7 +130,7 @@ func TestReadUsageMap_NoUsageKey(t *testing.T) { func TestReadUsageMap_InvalidUsageJSON(t *testing.T) { m := map[string]json.RawMessage{"usage": json.RawMessage(`"not an object"`)} - in, out, ok := readUsageMap(m) + _, _, ok := readUsageMap(m) if ok { t.Error("invalid usage JSON: ok should be false") } @@ -138,7 +138,7 @@ func TestReadUsageMap_InvalidUsageJSON(t *testing.T) { func TestReadUsageMap_ZeroUsage(t *testing.T) { m := map[string]json.RawMessage{"usage": json.RawMessage(`{"input_tokens": 0, "output_tokens": 0}`)} - in, out, ok := readUsageMap(m) + _, _, ok := readUsageMap(m) if ok { t.Error("zero usage: ok should be false") } -- 2.45.2 From a65cea7b66864ce3e0e2b1bb705448c363e4b254 Mon Sep 17 00:00:00 2001 From: dev-lead Date: Tue, 12 May 2026 19:44:22 -0700 Subject: [PATCH 11/11] fix: handle json null and empty array in extractToolTrace JSON null unmarshals to []byte("null") (4 bytes), not nil, so len(trace)==0 missed it. Empty array []byte("[]")==2 bytes was also returned unchanged. Add explicit string checks for both cases. Also fix TestExtractToolTrace_ValidNonEmpty: json.Marshal compacts spacing, so byte-exact comparison against spaced literal fails on round-trip. Use compact literal instead. Fixes mc#669 (null tool_trace panic path). --- workspace-server/internal/handlers/a2a_proxy_helpers.go | 2 +- workspace-server/internal/handlers/a2a_proxy_helpers_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/workspace-server/internal/handlers/a2a_proxy_helpers.go b/workspace-server/internal/handlers/a2a_proxy_helpers.go index ded26ec5..6e3058cb 100644 --- a/workspace-server/internal/handlers/a2a_proxy_helpers.go +++ b/workspace-server/internal/handlers/a2a_proxy_helpers.go @@ -410,7 +410,7 @@ func extractToolTrace(respBody []byte) json.RawMessage { return nil } trace, ok := meta["tool_trace"] - if !ok || len(trace) == 0 { + if !ok || len(trace) == 0 || string(trace) == "null" || string(trace) == "[]" { return nil } return trace diff --git a/workspace-server/internal/handlers/a2a_proxy_helpers_test.go b/workspace-server/internal/handlers/a2a_proxy_helpers_test.go index 273c6750..b3677cc1 100644 --- a/workspace-server/internal/handlers/a2a_proxy_helpers_test.go +++ b/workspace-server/internal/handlers/a2a_proxy_helpers_test.go @@ -94,7 +94,7 @@ func TestExtractToolTrace_EmptyArray(t *testing.T) { } func TestExtractToolTrace_ValidNonEmpty(t *testing.T) { - trace := []byte(`[{"name": "search", "result": "done"}]`) + trace := []byte(`[{"name":"search","result":"done"}]`) body, _ := json.Marshal(map[string]interface{}{ "result": map[string]interface{}{ "metadata": map[string]interface{}{ -- 2.45.2