[object Object]

← back to Cli Printing Press

fix(cli): drop MCP handler pre-marshal that base64-encoded mutating bodies (#1357)

1fc5f62bef9c21716e27c092545ed2ca8c6f1aca · 2026-05-13 20:57:16 -0700 · Trevin Chow

* fix(cli): drop MCP handler pre-marshal that base64-encoded mutating bodies

makeAPIHandler called json.Marshal(bodyArgs) before passing the result
to c.PostWithParams/PutWithParams/PatchWithParams. The generated
client.do() already json-marshals what it receives, and encoding/json
special-cases []byte into a base64-quoted string. Every MCP-driven
POST/PUT/PATCH was shipping payloads like "eyJ0b2tlbiI6Li4ufQ==" instead
of {"token":"..."}, which strict APIs reject. CLI handlers were
unaffected because they pass map[string]any straight through.

Forward bodyArgs directly. The bodyJSONOverride (json.RawMessage) path
is unchanged — RawMessage round-trips through json.Marshal verbatim.

TestMCPHandlerPassesBodyArgsMap pins both the positive form (each branch
forwards bodyArgs directly) and the negative form (no json.Marshal
intermediate). Five golden mcp/tools.go fixtures regenerate to match.

Fixes #1352.

* docs(cli): document MCP pre-marshal base64 wire-payload learning

Captures the Go encoding/json gotcha (special-case base64-encoding of
[]byte) that landed the #1352 fix, plus the body-as-any contract pattern
the MCP handler should follow, in docs/solutions/runtime-errors/.

Files touched

Diff

commit 1fc5f62bef9c21716e27c092545ed2ca8c6f1aca
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Wed May 13 20:57:16 2026 -0700

    fix(cli): drop MCP handler pre-marshal that base64-encoded mutating bodies (#1357)
    
    * fix(cli): drop MCP handler pre-marshal that base64-encoded mutating bodies
    
    makeAPIHandler called json.Marshal(bodyArgs) before passing the result
    to c.PostWithParams/PutWithParams/PatchWithParams. The generated
    client.do() already json-marshals what it receives, and encoding/json
    special-cases []byte into a base64-quoted string. Every MCP-driven
    POST/PUT/PATCH was shipping payloads like "eyJ0b2tlbiI6Li4ufQ==" instead
    of {"token":"..."}, which strict APIs reject. CLI handlers were
    unaffected because they pass map[string]any straight through.
    
    Forward bodyArgs directly. The bodyJSONOverride (json.RawMessage) path
    is unchanged — RawMessage round-trips through json.Marshal verbatim.
    
    TestMCPHandlerPassesBodyArgsMap pins both the positive form (each branch
    forwards bodyArgs directly) and the negative form (no json.Marshal
    intermediate). Five golden mcp/tools.go fixtures regenerate to match.
    
    Fixes #1352.
    
    * docs(cli): document MCP pre-marshal base64 wire-payload learning
    
    Captures the Go encoding/json gotcha (special-case base64-encoding of
    []byte) that landed the #1352 fix, plus the body-as-any contract pattern
    the MCP handler should follow, in docs/solutions/runtime-errors/.
---
 ...arshaled-body-base64-wire-payload-2026-05-13.md | 76 ++++++++++++++++++++++
 internal/generator/generator_test.go               | 61 +++++++++++++++++
 internal/generator/templates/mcp_tools.go.tmpl     | 11 ++--
 .../printing-press-rich-auth/internal/mcp/tools.go |  9 +--
 .../printing-press-golden/internal/mcp/tools.go    |  9 +--
 .../mcp-cloudflare/internal/mcp/tools.go           |  9 +--
 .../public-param-golden/internal/mcp/tools.go      |  9 +--
 .../tier-routing-golden/internal/mcp/tools.go      |  9 +--
 8 files changed, 156 insertions(+), 37 deletions(-)

diff --git a/docs/solutions/runtime-errors/mcp-handler-pre-marshaled-body-base64-wire-payload-2026-05-13.md b/docs/solutions/runtime-errors/mcp-handler-pre-marshaled-body-base64-wire-payload-2026-05-13.md
new file mode 100644
index 00000000..d91523af
--- /dev/null
+++ b/docs/solutions/runtime-errors/mcp-handler-pre-marshaled-body-base64-wire-payload-2026-05-13.md
@@ -0,0 +1,76 @@
+---
+title: "MCP handler pre-marshaled bodyArgs to []byte, causing base64-encoded JSON on every POST/PUT/PATCH"
+date: 2026-05-13
+category: runtime-errors
+module: internal/generator/templates
+problem_type: runtime_error
+component: tooling
+symptoms:
+  - "Every mutating MCP tool (POST/PUT/PATCH) sent the request body as a base64-encoded JSON string (e.g. \"eyJ0b2tlbiI6Li4ufQ==\") instead of the JSON object the spec described"
+  - "Strict upstream APIs (Pushover, Stripe, Twilio, Jira, Linear) rejected the malformed payload"
+  - "CLI-driven mutations worked correctly while MCP-driven mutations of the same endpoint failed — only the MCP path was affected"
+  - "Bug reproduced in 38 published CLIs that ship mutating MCP tools, traceable to a single template emission shape"
+root_cause: wrong_api
+resolution_type: code_fix
+severity: high
+tags: [mcp, json-marshal, base64-encoding, wire-format, template-emission, encoding-json-gotcha]
+---
+
+# MCP handler pre-marshaled bodyArgs to []byte, causing base64-encoded JSON on every POST/PUT/PATCH
+
+## Problem
+
+The generator's `mcp_tools.go.tmpl` emitted `body, _ := json.Marshal(bodyArgs)` and then passed the resulting `[]byte` to `c.PostWithParams(path, params, body)` (and the PUT/PATCH equivalents). The generated `client.do()` already calls `json.Marshal(body)` on whatever it receives. When `body` arrived as a `[]byte`, Go's `encoding/json` applied its special-case behavior for byte slices and base64-encoded the bytes into a quoted JSON string — so every mutating MCP tool sent payloads shaped like `"eyJ0b2tlbiI6Li4ufQ=="` instead of `{"token":"..."}`. Strict APIs rejected the malformed bodies.
+
+## Symptoms
+
+- POST/PUT/PATCH endpoints invoked through the MCP server returned 4xx errors from strict APIs even with valid arguments.
+- The same endpoint invoked through the generated CLI worked correctly, because CLI handlers passed `map[string]any` straight to `c.Post(...)`.
+- Network captures showed a quoted base64 string where the JSON request object should have been.
+- The bug was visible in 38 published CLIs that wrap mutating MCP tools (a `grep -l 'body, _ := json.Marshal(bodyArgs)' library/**/internal/mcp/tools.go` in the public-library mirror returned 38 matches), confirming the defect lived in the shared template, not any individual CLI.
+
+## What Didn't Work
+
+- Inspecting per-CLI generated `tools.go` files for typos — the shape was identical across every affected CLI because it came from a single template.
+- Looking for an issue in the OpenAPI parser or the body-binding code — the binding step correctly assembled `bodyArgs` as `map[string]any`. The corruption happened one step later, in the call to the client.
+- Adding a `Content-Type: application/json` header to the request — the header was already set; the issue was the body content, not the framing.
+
+## Solution
+
+Drop the intermediate `json.Marshal` in each of the three mutating switch branches and forward `bodyArgs` directly. The downstream `client.do()` marshals whatever it receives.
+
+```go
+// internal/generator/templates/mcp_tools.go.tmpl
+// Before:
+case "POST":
+    // ... multipart / form / bodyJSONOverride branches with `break` guards ...
+    body, _ := json.Marshal(bodyArgs)
+    data, _, err = c.PostWithParams(path, params, body)
+
+// After:
+case "POST":
+    // ... multipart / form / bodyJSONOverride branches with `break` guards ...
+    data, _, err = c.PostWithParams(path, params, bodyArgs)
+```
+
+The `bodyJSONOverride` path is unchanged because it already passed a `json.RawMessage`, which round-trips through `json.Marshal` verbatim via its `MarshalJSON` method.
+
+Regression test in `internal/generator/generator_test.go::TestMCPHandlerPassesBodyArgsMap` asserts every branch forwards `bodyArgs` directly and that `json.Marshal(bodyArgs)` does not appear in the generated output. Five golden fixtures under `testdata/golden/expected/.../internal/mcp/tools.go` were regenerated to match the new emission.
+
+## Why This Works
+
+`encoding/json` treats `[]byte` as a special case: it base64-encodes the bytes into a JSON string, because that is the standard way to round-trip arbitrary binary data through JSON. The `client.do()` helper accepts `body any` and marshals it internally — that contract works correctly for `map[string]any`, structs, slices, and `json.RawMessage` (which opts out of marshaling via its custom `MarshalJSON`). Passing an already-marshaled `[]byte` to a body-as-any helper double-encodes by design.
+
+The fix aligns the MCP path with what the CLI path already did: the layer that owns the wire format (the client) is the only layer that marshals, and producers hand it the live Go value, not a serialized intermediate.
+
+## Prevention
+
+- When a helper takes `body any` and marshals internally, never pass it a `[]byte` you produced with `json.Marshal`. Either pass the raw Go value, or convert to `json.RawMessage` if you genuinely need to forward pre-serialized JSON.
+- Pin the template's emission shape with a string-contains regression test that asserts both the positive ("body forwarded directly") and negative ("no `json.Marshal(bodyArgs)` pre-marshal") forms. Generator templates fail silently — golden fixtures catch drift, but a focused unit test makes the WHY recoverable.
+- When fixing a template-level defect, sweep the published library for the same shape (`grep -l '<defect signature>' library/**/internal/...`) so the reprint/republish backlog can be sized at filing time. The Printing Press intentionally separates machine fixes from per-CLI reprints; counting affected CLIs early prevents surprise after the fix lands.
+- Go's `encoding/json` has other type-driven special cases (e.g., `time.Time.MarshalJSON`, `*url.URL.MarshalJSON` returning the string form). When a downstream `Marshal` call sees a different result than expected, check whether the input type has a `MarshalJSON` method or is one of the stdlib special cases (`[]byte`, `Marshaler`, `encoding.TextMarshaler`).
+
+## Related Issues
+
+- [#1352](https://github.com/mvanhorn/cli-printing-press/issues/1352) — Original bug report surfaced by Greptile during review of [printing-press-library#511](https://github.com/mvanhorn/printing-press-library/pull/511), with the 38-CLI grep and a hand-patch on the `feat/pushover` branch ([bdd07db4](https://github.com/mvanhorn/printing-press-library/commit/bdd07db4)).
+- [`docs/solutions/logic-errors/mcp-handler-conflates-path-and-query-positional-params-2026-05-05.md`](../logic-errors/mcp-handler-conflates-path-and-query-positional-params-2026-05-05.md) — Different bug in the same MCP handler template (URL path placeholder vs CLI positional query arg). Same file, different root cause.
diff --git a/internal/generator/generator_test.go b/internal/generator/generator_test.go
index d311f1dc..9f17e63e 100644
--- a/internal/generator/generator_test.go
+++ b/internal/generator/generator_test.go
@@ -9172,6 +9172,67 @@ func TestGeneratePublicParamNamesAcrossCLISurfaces(t *testing.T) {
 	assert.Contains(t, skill, `public-params-pp-cli stores create --store-code example-value`)
 }
 
+// TestMCPHandlerPassesBodyArgsMap pins that POST/PUT/PATCH branches forward
+// the bodyArgs map directly to the client, not via a pre-marshal. The client's
+// do() json.Marshals what it receives; passing []byte causes encoding/json to
+// base64-encode it into a quoted string, which strict APIs reject.
+func TestMCPHandlerPassesBodyArgsMap(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := minimalSpec("mcp-body-passthrough")
+	apiSpec.Resources["widgets"] = spec.Resource{
+		Description: "Widgets",
+		Endpoints: map[string]spec.Endpoint{
+			"create": {
+				Method:      "POST",
+				Path:        "/widgets",
+				Description: "Create a widget",
+				Body: []spec.Param{
+					{Name: "label", Type: "string", Required: true, Description: "Widget label"},
+				},
+			},
+			"replace": {
+				Method:      "PUT",
+				Path:        "/widgets/{id}",
+				Description: "Replace a widget",
+				Params: []spec.Param{
+					{Name: "id", Type: "string", Required: true, Description: "Widget id"},
+				},
+				Body: []spec.Param{
+					{Name: "label", Type: "string", Required: true, Description: "Widget label"},
+				},
+			},
+			"update": {
+				Method:      "PATCH",
+				Path:        "/widgets/{id}",
+				Description: "Update a widget",
+				Params: []spec.Param{
+					{Name: "id", Type: "string", Required: true, Description: "Widget id"},
+				},
+				Body: []spec.Param{
+					{Name: "label", Type: "string", Required: true, Description: "Widget label"},
+				},
+			},
+		},
+	}
+
+	outputDir := filepath.Join(t.TempDir(), naming.CLI(apiSpec.Name))
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	mcpSource := readGeneratedFile(t, outputDir, "internal", "mcp", "tools.go")
+
+	assert.NotContains(t, mcpSource, "json.Marshal(bodyArgs)",
+		"MCP handler must not pre-marshal bodyArgs: client.do() json.Marshals what it receives, "+
+			"so a []byte arrives base64-encoded on the wire and strict APIs reject the payload")
+
+	assert.Contains(t, mcpSource, "c.PostWithParams(path, params, bodyArgs)",
+		"POST branch must forward bodyArgs directly to the client")
+	assert.Contains(t, mcpSource, "c.PutWithParams(path, params, bodyArgs)",
+		"PUT branch must forward bodyArgs directly to the client")
+	assert.Contains(t, mcpSource, "c.PatchWithParams(path, params, bodyArgs)",
+		"PATCH branch must forward bodyArgs directly to the client")
+}
+
 func TestGeneratePublicParamNamesInPromotedExamples(t *testing.T) {
 	t.Parallel()
 
diff --git a/internal/generator/templates/mcp_tools.go.tmpl b/internal/generator/templates/mcp_tools.go.tmpl
index f2d8a150..eec0339d 100644
--- a/internal/generator/templates/mcp_tools.go.tmpl
+++ b/internal/generator/templates/mcp_tools.go.tmpl
@@ -261,7 +261,7 @@ func makeAPIHandler(method, pathTemplate string, bindings []mcpParamBinding, pos
 {{- if $hasBodyJSONFallback}}
 		// bodyJSONOverride holds an opaque body for endpoints whose request
 		// schema is a oneOf/anyOf and was wired with a single body_json input.
-		// When set, it replaces the marshaled bodyArgs map for POST/PUT/PATCH.
+		// When set, it replaces the bodyArgs map for POST/PUT/PATCH.
 		var bodyJSONOverride json.RawMessage
 {{- end}}
 		for _, binding := range bindings {
@@ -374,8 +374,7 @@ func makeAPIHandler(method, pathTemplate string, bindings []mcpParamBinding, pos
 				break
 			}
 {{- end}}
-			body, _ := json.Marshal(bodyArgs)
-			data, _, err = c.PostWithParams(path, params, body)
+			data, _, err = c.PostWithParams(path, params, bodyArgs)
 		case "PUT":
 {{- if $hasMultipartRequest}}
 			if multipart {
@@ -395,8 +394,7 @@ func makeAPIHandler(method, pathTemplate string, bindings []mcpParamBinding, pos
 				break
 			}
 {{- end}}
-			body, _ := json.Marshal(bodyArgs)
-			data, _, err = c.PutWithParams(path, params, body)
+			data, _, err = c.PutWithParams(path, params, bodyArgs)
 		case "PATCH":
 {{- if $hasMultipartRequest}}
 			if multipart {
@@ -416,8 +414,7 @@ func makeAPIHandler(method, pathTemplate string, bindings []mcpParamBinding, pos
 				break
 			}
 {{- end}}
-			body, _ := json.Marshal(bodyArgs)
-			data, _, err = c.PatchWithParams(path, params, body)
+			data, _, err = c.PatchWithParams(path, params, bodyArgs)
 		case "DELETE":
 			data, _, err = c.DeleteWithParams(path, params)
 		default:
diff --git a/testdata/golden/expected/generate-golden-api-rich-auth/printing-press-rich-auth/internal/mcp/tools.go b/testdata/golden/expected/generate-golden-api-rich-auth/printing-press-rich-auth/internal/mcp/tools.go
index 47e49c1a..98853611 100644
--- a/testdata/golden/expected/generate-golden-api-rich-auth/printing-press-rich-auth/internal/mcp/tools.go
+++ b/testdata/golden/expected/generate-golden-api-rich-auth/printing-press-rich-auth/internal/mcp/tools.go
@@ -132,14 +132,11 @@ func makeAPIHandler(method, pathTemplate string, bindings []mcpParamBinding, pos
 		case "GET":
 			data, err = c.Get(path, params)
 		case "POST":
-			body, _ := json.Marshal(bodyArgs)
-			data, _, err = c.PostWithParams(path, params, body)
+			data, _, err = c.PostWithParams(path, params, bodyArgs)
 		case "PUT":
-			body, _ := json.Marshal(bodyArgs)
-			data, _, err = c.PutWithParams(path, params, body)
+			data, _, err = c.PutWithParams(path, params, bodyArgs)
 		case "PATCH":
-			body, _ := json.Marshal(bodyArgs)
-			data, _, err = c.PatchWithParams(path, params, body)
+			data, _, err = c.PatchWithParams(path, params, bodyArgs)
 		case "DELETE":
 			data, _, err = c.DeleteWithParams(path, params)
 		default:
diff --git a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/mcp/tools.go b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/mcp/tools.go
index ac0cb42d..560ef840 100644
--- a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/mcp/tools.go
+++ b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/mcp/tools.go
@@ -264,22 +264,19 @@ func makeAPIHandler(method, pathTemplate string, bindings []mcpParamBinding, pos
 				data, _, err = c.PostMultipartWithParams(path, params, multipartFields, multipartFileFields)
 				break
 			}
-			body, _ := json.Marshal(bodyArgs)
-			data, _, err = c.PostWithParams(path, params, body)
+			data, _, err = c.PostWithParams(path, params, bodyArgs)
 		case "PUT":
 			if multipart {
 				data, _, err = c.PutMultipartWithParams(path, params, multipartFields, multipartFileFields)
 				break
 			}
-			body, _ := json.Marshal(bodyArgs)
-			data, _, err = c.PutWithParams(path, params, body)
+			data, _, err = c.PutWithParams(path, params, bodyArgs)
 		case "PATCH":
 			if multipart {
 				data, _, err = c.PatchMultipartWithParams(path, params, multipartFields, multipartFileFields)
 				break
 			}
-			body, _ := json.Marshal(bodyArgs)
-			data, _, err = c.PatchWithParams(path, params, body)
+			data, _, err = c.PatchWithParams(path, params, bodyArgs)
 		case "DELETE":
 			data, _, err = c.DeleteWithParams(path, params)
 		default:
diff --git a/testdata/golden/expected/generate-mcp-api/mcp-cloudflare/internal/mcp/tools.go b/testdata/golden/expected/generate-mcp-api/mcp-cloudflare/internal/mcp/tools.go
index 6cd93040..e49b33b9 100644
--- a/testdata/golden/expected/generate-mcp-api/mcp-cloudflare/internal/mcp/tools.go
+++ b/testdata/golden/expected/generate-mcp-api/mcp-cloudflare/internal/mcp/tools.go
@@ -126,14 +126,11 @@ func makeAPIHandler(method, pathTemplate string, bindings []mcpParamBinding, pos
 		case "GET":
 			data, err = c.Get(path, params)
 		case "POST":
-			body, _ := json.Marshal(bodyArgs)
-			data, _, err = c.PostWithParams(path, params, body)
+			data, _, err = c.PostWithParams(path, params, bodyArgs)
 		case "PUT":
-			body, _ := json.Marshal(bodyArgs)
-			data, _, err = c.PutWithParams(path, params, body)
+			data, _, err = c.PutWithParams(path, params, bodyArgs)
 		case "PATCH":
-			body, _ := json.Marshal(bodyArgs)
-			data, _, err = c.PatchWithParams(path, params, body)
+			data, _, err = c.PatchWithParams(path, params, bodyArgs)
 		case "DELETE":
 			data, _, err = c.DeleteWithParams(path, params)
 		default:
diff --git a/testdata/golden/expected/generate-public-param-names/public-param-golden/internal/mcp/tools.go b/testdata/golden/expected/generate-public-param-names/public-param-golden/internal/mcp/tools.go
index dc580b5b..25b3c856 100644
--- a/testdata/golden/expected/generate-public-param-names/public-param-golden/internal/mcp/tools.go
+++ b/testdata/golden/expected/generate-public-param-names/public-param-golden/internal/mcp/tools.go
@@ -131,14 +131,11 @@ func makeAPIHandler(method, pathTemplate string, bindings []mcpParamBinding, pos
 		case "GET":
 			data, err = c.Get(path, params)
 		case "POST":
-			body, _ := json.Marshal(bodyArgs)
-			data, _, err = c.PostWithParams(path, params, body)
+			data, _, err = c.PostWithParams(path, params, bodyArgs)
 		case "PUT":
-			body, _ := json.Marshal(bodyArgs)
-			data, _, err = c.PutWithParams(path, params, body)
+			data, _, err = c.PutWithParams(path, params, bodyArgs)
 		case "PATCH":
-			body, _ := json.Marshal(bodyArgs)
-			data, _, err = c.PatchWithParams(path, params, body)
+			data, _, err = c.PatchWithParams(path, params, bodyArgs)
 		case "DELETE":
 			data, _, err = c.DeleteWithParams(path, params)
 		default:
diff --git a/testdata/golden/expected/generate-tier-routing-api/tier-routing-golden/internal/mcp/tools.go b/testdata/golden/expected/generate-tier-routing-api/tier-routing-golden/internal/mcp/tools.go
index 7522b562..aa25d051 100644
--- a/testdata/golden/expected/generate-tier-routing-api/tier-routing-golden/internal/mcp/tools.go
+++ b/testdata/golden/expected/generate-tier-routing-api/tier-routing-golden/internal/mcp/tools.go
@@ -151,14 +151,11 @@ func makeAPIHandler(method, pathTemplate, tier string, bindings []mcpParamBindin
 		case "GET":
 			data, err = c.Get(path, params)
 		case "POST":
-			body, _ := json.Marshal(bodyArgs)
-			data, _, err = c.PostWithParams(path, params, body)
+			data, _, err = c.PostWithParams(path, params, bodyArgs)
 		case "PUT":
-			body, _ := json.Marshal(bodyArgs)
-			data, _, err = c.PutWithParams(path, params, body)
+			data, _, err = c.PutWithParams(path, params, bodyArgs)
 		case "PATCH":
-			body, _ := json.Marshal(bodyArgs)
-			data, _, err = c.PatchWithParams(path, params, body)
+			data, _, err = c.PatchWithParams(path, params, bodyArgs)
 		case "DELETE":
 			data, _, err = c.DeleteWithParams(path, params)
 		default:

← 0f8dc989 fix(cli): strip embedded .git/ and .gitmodules in pipeline.C  ·  back to Cli Printing Press  ·  fix(cli): percent-encode path param values in replacePathPar 77fce43f →