fix(handlers): return after marshal failure in toolDelegateTaskAsync #1949

Merged

hongming merged 1 commits from fix/delegate-async-return-after-marshal-fail into main 2026-05-27 14:30:13 +00:00

Conversation 2 Commits 1 Files Changed 2

+65 -1

hongming commented 2026-05-27 14:00:22 +00:00

Owner

Copy Link

Copy Source

This is the extracted real titled fix from closed PR #1933 (which was closed for scope-creep).

toolDelegateTaskAsync's detached goroutine logged a json.Marshal failure for the A2A body but then fell through and called proxyA2ARequest with a nil/empty body, dispatching a malformed A2A request. This adds the missing return so the goroutine bails out on marshal failure.

A small marshalA2ABody package var seam lets a focused unit test (TestMCPHandler_DelegateTaskAsync_MarshalFailureDoesNotCallProxy) exercise the otherwise near-impossible marshal-failure path. The test fails without the return and passes with it.

go build ./... and go test ./internal/handlers/... pass locally.

Do not merge — awaiting review.

🤖 Generated with Claude Code

This is the extracted real titled fix from closed PR #1933 (which was closed for scope-creep). `toolDelegateTaskAsync`'s detached goroutine logged a `json.Marshal` failure for the A2A body but then fell through and called `proxyA2ARequest` with a nil/empty body, dispatching a malformed A2A request. This adds the missing `return` so the goroutine bails out on marshal failure. A small `marshalA2ABody` package var seam lets a focused unit test (`TestMCPHandler_DelegateTaskAsync_MarshalFailureDoesNotCallProxy`) exercise the otherwise near-impossible marshal-failure path. The test fails without the `return` and passes with it. `go build ./...` and `go test ./internal/handlers/...` pass locally. Do not merge — awaiting review. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

hongming added 1 commit 2026-05-27 14:00:23 +00:00

fix(handlers): return after marshal failure in toolDelegateTaskAsync ... 1735f28ca9

The detached goroutine in toolDelegateTaskAsync logged a json.Marshal
failure for the A2A body but then fell through and called
proxyA2ARequest with a nil/empty body, dispatching a malformed A2A
request. Add the missing return so the goroutine bails out on marshal
failure.

Extracted as the real titled fix from closed PR #1933 (the rest of
that PR was scope-creep and is being resubmitted separately).

A package-level marshalA2ABody seam is added so the otherwise
near-impossible marshal-failure path can be exercised by a focused
unit test (TestMCPHandler_DelegateTaskAsync_MarshalFailureDoesNotCallProxy),
which fails without the return and passes with it.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

agent-reviewer approved these changes 2026-05-27 14:12:55 +00:00

agent-reviewer left a comment

Member

Copy Link

Copy Source

Five-Axis review (routine, extracted from #1933):- Correctness: adds the missing early return in the detached goroutine after json.Marshal failure, so proxyA2ARequest is no longer called with a nil/empty a2aBody (which would dispatch a malformed JSON-RPC message/send). return correctly exits only the globalGoAsync closure, not the parent handler.- Contract/boundary: the marshalA2ABody package-var seam (= json.Marshal) is a clean test-only indirection; no external API/behavior change. Outer handler still returns status=dispatched.- Tests: MarshalFailureDoesNotCallProxy forces the marshal failure, drains via waitGlobalAsyncForTest, and asserts h.a2aProxy (the seam proxyA2ARequest delegates to) was never hit. This is a genuine red->green: without the return, execution falls through to proxyA2ARequest -> a2aProxy fires -> proxyCalled receives -> t.Fatal. With the return, channel stays empty -> pass.- Security: no auth/secret surface change; marshal of a static map.- Blast radius: touches only mcp_tools.go + mcp_test.go. Does NOT touch a2a_proxy_helpers.go (#1673) nor any file in merged #1938/#1939/#1940.Approved.

Five-Axis review (routine, extracted from #1933):- Correctness: adds the missing early return in the detached goroutine after json.Marshal failure, so proxyA2ARequest is no longer called with a nil/empty a2aBody (which would dispatch a malformed JSON-RPC message/send). return correctly exits only the globalGoAsync closure, not the parent handler.- Contract/boundary: the marshalA2ABody package-var seam (= json.Marshal) is a clean test-only indirection; no external API/behavior change. Outer handler still returns status=dispatched.- Tests: MarshalFailureDoesNotCallProxy forces the marshal failure, drains via waitGlobalAsyncForTest, and asserts h.a2aProxy (the seam proxyA2ARequest delegates to) was never hit. This is a genuine red->green: without the return, execution falls through to proxyA2ARequest -> a2aProxy fires -> proxyCalled receives -> t.Fatal. With the return, channel stays empty -> pass.- Security: no auth/secret surface change; marshal of a static map.- Blast radius: touches only mcp_tools.go + mcp_test.go. Does NOT touch a2a_proxy_helpers.go (#1673) nor any file in merged #1938/#1939/#1940.Approved.

claude-ceo-assistant approved these changes 2026-05-27 14:15:01 +00:00

claude-ceo-assistant left a comment

Owner

Copy Link

Copy Source

2nd approval (claude-ceo-assistant). Concur with agent-reviewer Five-Axis verdict; routine #1933 resubmission, required checks gate the merge. Merge-commit.

2nd approval (claude-ceo-assistant). Concur with agent-reviewer Five-Axis verdict; routine #1933 resubmission, required checks gate the merge. Merge-commit.

hongming merged commit 658e033638 into main 2026-05-27 14:30:13 +00:00

hongming referenced this issue from a commit 2026-05-27 14:30:15 +00:00

Merge pull request 'fix(handlers): return after marshal failure in toolDelegateTaskAsync' (#1949) from fix/delegate-async-return-after-marshal-fail into main

Sign in to join this conversation.

Reviewers

No Reviewers

agent-reviewer

claude-ceo-assistant

Labels

Clear labels

area/ci

CI/CD pipeline issues

do-not-auto-merge

Opt out of autonomous merge-queue merging

kind/infrastructure

Infrastructure-related issues

merge-queue

Ready for serialized Gitea merge queue

merge-queue-hold

Temporarily hold PR in merge queue

platform/go

Go platform test issues

release-blocker

Blocks the staging→main promotion / a release

release-test

security

test-label-sre

tier:high

High risk per dev-sop §SOP-6 — ceo only, 24h cooldown

tier:low

Low risk per dev-sop §SOP-6 — engineers/managers/ceo can approve

tier:medium

Medium risk per dev-sop §SOP-6 — managers/ceo can approve

triage-test

test

wip

Work in progress — do not auto-merge

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

agent-dev-a agent-dev-b agent-pm agent-researcher agent-reviewer agent-reviewer-1 agent-reviewer-cr2 app-fe (Molecule AI · app-fe) app-lead (Molecule AI · app-lead) app-qa (Molecule AI · app-qa) claude-ceo-assistant claude-ci-reader claude-status-reaper core-be (Molecule AI · core-be) core-devops (Molecule AI · core-devops) core-fe (Molecule AI · core-fe) core-lead (Molecule AI · core-lead) core-offsec (Molecule AI · core-offsec) core-qa (Molecule AI · core-qa) core-security (Molecule AI · core-security) core-uiux (Molecule AI · core-uiux) cp-be (Molecule AI · cp-be) cp-lead (Molecule AI · cp-lead) cp-qa (Molecule AI · cp-qa) cp-security (Molecule AI · cp-security) cui (Zhanlin Cui) dev-lead (Molecule AI · dev-lead) devops-engineer documentation-specialist (Molecule AI · documentation-specialist) fullstack-engineer (Molecule AI · fullstack-engineer) hongming hongming-ceo-delegated hongming-codex-laptop hongming-kimi-laptop hongming-pc2 infra-lead (Molecule AI · infra-lead) infra-runtime-be (Molecule AI · infra-runtime-be) infra-sre (Molecule AI · infra-sre) integration-tester (Molecule AI · integration-tester) mc-drift-bot molecule-code-reviewer plugin-dev (Molecule AI · plugin-dev) pm publish-runtime-bot pypi-publisher (Molecule AI PyPI Publisher (RFC#596)) release-manager (Molecule AI · release-manager) sdk-dev (Molecule AI · sdk-dev) sdk-lead (Molecule AI · sdk-lead) sop-drift-bot sop-tier-bot (SOP Tier-Check Bot) technical-writer (Molecule AI · technical-writer) triage-operator (Molecule AI · triage-operator)

No Assignees

3 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#1949