fix(mcp): emit all three tool annotation hints for ChatGPT Apps

internal/mcpserver/annotations.go

@@ -0,0 +1,159 @@
+package mcpserver
+
+import (
+	"bytes"
+	"encoding/json"
+	"io"
+	"net/http"
+	"strconv"
+)
+
+// ensureReadOnlyHint guarantees every tool in a tools/list response carries an
+// explicit readOnlyHint. The Go MCP SDK marshals ToolAnnotations.ReadOnlyHint
+// with `omitempty`, so a write tool's correct value (readOnlyHint:false) is
+// dropped from the wire. OpenAI's ChatGPT Apps submission, however, requires
+// readOnlyHint, openWorldHint, and destructiveHint to be present (true or
+// false) on every tool. This shim re-adds the spec-default readOnlyHint:false
+// wherever a tool's annotations object omits it (i.e. our save_recipe tool).
+//
+// Only tools/list responses are buffered and rewritten; every other request —
+// including streaming tool calls — passes through untouched so streaming is
+// preserved.
+func ensureReadOnlyHint(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodPost {
+			next.ServeHTTP(w, r)
+			return
+		}
+		// Peek the request body to see if this is a tools/list call, then
+		// restore it for the wrapped handler.
+		body, err := io.ReadAll(io.LimitReader(r.Body, 1<<20))
+		_ = r.Body.Close()
+		r.Body = io.NopCloser(bytes.NewReader(body))
+		if err != nil || !bytes.Contains(body, []byte(`"tools/list"`)) {
+			next.ServeHTTP(w, r)
+			return
+		}
+
+		rec := &bufferingWriter{header: make(http.Header), body: &bytes.Buffer{}}
+		next.ServeHTTP(rec, r)
+		out := fixToolsListBody(rec.body.Bytes())
+
+		h := w.Header()
+		for k, v := range rec.header {
+			h[k] = v
+		}
+		h.Set("Content-Length", strconv.Itoa(len(out)))
+		status := rec.status
+		if status == 0 {
+			status = http.StatusOK
+		}
+		w.WriteHeader(status)
+		_, _ = w.Write(out)
+	})
+}
+
+// bufferingWriter captures a handler's response so it can be rewritten before
+// being flushed to the real client.
+type bufferingWriter struct {
+	header http.Header
+	status int
+	body   *bytes.Buffer
+}
+
+func (b *bufferingWriter) Header() http.Header         { return b.header }
+func (b *bufferingWriter) WriteHeader(code int)        { b.status = code }
+func (b *bufferingWriter) Write(p []byte) (int, error) { return b.body.Write(p) }
+
+// fixToolsListBody rewrites a tools/list response body (either a plain JSON
+// response or an SSE-framed one) to backfill missing readOnlyHint annotations.
+func fixToolsListBody(body []byte) []byte {
+	trimmed := bytes.TrimLeft(body, " \t\r\n")
+	if len(trimmed) > 0 && trimmed[0] == '{' {
+		if fixed, ok := fixReadOnlyHintJSON(body); ok {
+			return fixed
+		}
+		return body
+	}
+	// SSE framing (text/event-stream): rewrite each data: line's JSON payload.
+	if bytes.Contains(body, []byte("data:")) {
+		lines := bytes.Split(body, []byte("\n"))
+		changed := false
+		for i, ln := range lines {
+			t := bytes.TrimLeft(ln, " \t")
+			if !bytes.HasPrefix(t, []byte("data:")) {
+				continue
+			}
+			payload := bytes.TrimSpace(t[len("data:"):])
+			if fixed, ok := fixReadOnlyHintJSON(payload); ok {
+				lines[i] = append([]byte("data: "), fixed...)
+				changed = true
+			}
+		}
+		if changed {
+			return bytes.Join(lines, []byte("\n"))
+		}
+	}
+	return body
+}
+
+// fixReadOnlyHintJSON parses a single JSON-RPC response object and, for every
+// tool in result.tools that has an annotations object without a readOnlyHint
+// key, adds readOnlyHint:false. It returns the (possibly rewritten) payload and
+// whether any change was made. On any parse failure it returns the input
+// unchanged so a malformed body is never corrupted.
+func fixReadOnlyHintJSON(payload []byte) ([]byte, bool) {
+	var msg map[string]json.RawMessage
+	if err := json.Unmarshal(payload, &msg); err != nil {
+		return payload, false
+	}
+	var result map[string]json.RawMessage
+	if err := json.Unmarshal(msg["result"], &result); err != nil {
+		return payload, false
+	}
+	var tools []map[string]json.RawMessage
+	if err := json.Unmarshal(result["tools"], &tools); err != nil {
+		return payload, false
+	}
+
+	changed := false
+	for _, tool := range tools {
+		annRaw, ok := tool["annotations"]
+		if !ok {
+			continue
+		}
+		var ann map[string]json.RawMessage
+		if err := json.Unmarshal(annRaw, &ann); err != nil {
+			continue
+		}
+		if _, has := ann["readOnlyHint"]; has {
+			continue
+		}
+		ann["readOnlyHint"] = json.RawMessage("false")
+		newAnn, err := json.Marshal(ann)
+		if err != nil {
+			continue
+		}
+		tool["annotations"] = newAnn
+		changed = true
+	}
+	if !changed {
+		return payload, false
+	}
+
+	newTools, err := json.Marshal(tools)
+	if err != nil {
+		return payload, false
+	}
+	result["tools"] = newTools
+	newResult, err := json.Marshal(result)
+	if err != nil {
+		return payload, false
+	}
+	msg["result"] = newResult
+	out, err := json.Marshal(msg)
+	if err != nil {
+		return payload, false
+	}
+	return out, true
+}

internal/mcpserver/annotations_test.go

@@ -0,0 +1,129 @@
+package mcpserver
+
+import (
+	"encoding/json"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+)
+
+// toolAnnotations unmarshals a JSON-RPC tools/list body and returns each tool's
+// annotations keyed by tool name.
+func toolAnnotations(t *testing.T, body []byte) map[string]map[string]any {
+	t.Helper()
+	var parsed struct {
+		Result struct {
+			Tools []struct {
+				Name        string         `json:"name"`
+				Annotations map[string]any `json:"annotations"`
+			} `json:"tools"`
+		} `json:"result"`
+	}
+	if err := json.Unmarshal(body, &parsed); err != nil {
+		t.Fatalf("unmarshal result: %v\nbody: %s", err, body)
+	}
+	out := make(map[string]map[string]any)
+	for _, tool := range parsed.Result.Tools {
+		out[tool.Name] = tool.Annotations
+	}
+	return out
+}
+
+func TestFixReadOnlyHintJSON_BackfillsMissing(t *testing.T) {
+	in := `{"jsonrpc":"2.0","id":1,"result":{"tools":[` +
+		`{"name":"save_recipe","annotations":{"destructiveHint":false,"openWorldHint":true}},` +
+		`{"name":"get_recipe","annotations":{"readOnlyHint":true,"destructiveHint":false,"openWorldHint":false}}` +
+		`]}}`
+
+	out, changed := fixReadOnlyHintJSON([]byte(in))
+	if !changed {
+		t.Fatal("expected the body to be changed")
+	}
+	ann := toolAnnotations(t, out)
+
+	save := ann["save_recipe"]
+	if v, ok := save["readOnlyHint"].(bool); !ok || v != false {
+		t.Fatalf("save_recipe readOnlyHint = %v (%T), want false", save["readOnlyHint"], save["readOnlyHint"])
+	}
+	// Its other annotations must survive intact.
+	if v, ok := save["destructiveHint"].(bool); !ok || v != false {
+		t.Fatalf("save_recipe destructiveHint = %v, want false", save["destructiveHint"])
+	}
+	if v, ok := save["openWorldHint"].(bool); !ok || v != true {
+		t.Fatalf("save_recipe openWorldHint = %v, want true", save["openWorldHint"])
+	}
+	// A tool that already declares readOnlyHint keeps its value.
+	if v, ok := ann["get_recipe"]["readOnlyHint"].(bool); !ok || v != true {
+		t.Fatalf("get_recipe readOnlyHint = %v, want true", ann["get_recipe"]["readOnlyHint"])
+	}
+}
+
+func TestFixReadOnlyHintJSON_NoChangeWhenComplete(t *testing.T) {
+	in := `{"jsonrpc":"2.0","id":1,"result":{"tools":[` +
+		`{"name":"get_recipe","annotations":{"readOnlyHint":true,"destructiveHint":false,"openWorldHint":false}}` +
+		`]}}`
+	if _, changed := fixReadOnlyHintJSON([]byte(in)); changed {
+		t.Fatal("expected no change when readOnlyHint already present")
+	}
+}
+
+func TestFixReadOnlyHintJSON_MalformedUntouched(t *testing.T) {
+	in := []byte(`not json at all`)
+	out, changed := fixReadOnlyHintJSON(in)
+	if changed || string(out) != string(in) {
+		t.Fatal("malformed body must be returned unchanged")
+	}
+}
+
+func TestFixToolsListBody_SSE(t *testing.T) {
+	body := "event: message\n" +
+		`data: {"jsonrpc":"2.0","id":1,"result":{"tools":[{"name":"save_recipe","annotations":{"openWorldHint":true}}]}}` +
+		"\n\n"
+	out := fixToolsListBody([]byte(body))
+	if !strings.Contains(string(out), `"readOnlyHint":false`) {
+		t.Fatalf("expected readOnlyHint backfilled in SSE data line, got: %s", out)
+	}
+	if !strings.HasPrefix(string(out), "event: message\n") {
+		t.Fatalf("SSE framing not preserved: %s", out)
+	}
+}
+
+func TestEnsureReadOnlyHint_Middleware(t *testing.T) {
+	// Stub MCP handler that returns a tools/list response missing readOnlyHint.
+	stub := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = io.WriteString(w, `{"jsonrpc":"2.0","id":1,"result":{"tools":[`+
+			`{"name":"save_recipe","annotations":{"destructiveHint":false,"openWorldHint":true}}`+
+			`]}}`)
+	})
+	h := ensureReadOnlyHint(stub)
+
+	req := httptest.NewRequest(http.MethodPost, "/mcp",
+		strings.NewReader(`{"jsonrpc":"2.0","id":1,"method":"tools/list"}`))
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, req)
+
+	ann := toolAnnotations(t, rec.Body.Bytes())
+	if v, ok := ann["save_recipe"]["readOnlyHint"].(bool); !ok || v != false {
+		t.Fatalf("middleware did not backfill readOnlyHint:false, got: %s", rec.Body.Bytes())
+	}
+}
+
+func TestEnsureReadOnlyHint_PassesThroughNonToolsList(t *testing.T) {
+	const raw = `{"jsonrpc":"2.0","id":1,"result":{"content":[]}}`
+	stub := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		_, _ = io.WriteString(w, raw)
+	})
+	h := ensureReadOnlyHint(stub)
+
+	req := httptest.NewRequest(http.MethodPost, "/mcp",
+		strings.NewReader(`{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"save_recipe"}}`))
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, req)
+
+	if rec.Body.String() != raw {
+		t.Fatalf("tools/call body must pass through untouched, got: %s", rec.Body.String())
+	}
+}

internal/mcpserver/server.go

@@ -110,5 +110,5 @@ func NewHandler(cfg *config.Config, deps *Deps) http.Handler {
 
 	return auth.RequireBearerToken(verifier, &auth.RequireBearerTokenOptions{
 		ResourceMetadataURL: deps.OAuth.Issuer() + "/.well-known/oauth-protected-resource/mcp",
-	})(mcpHandler)
+	})(ensureReadOnlyHint(mcpHandler))
 }

internal/mcpserver/tools.go

@@ -341,8 +341,9 @@ func registerTools(server *mcp.Server, deps *Deps) {
 		),
 		// Read-only: queries the open web; makes no changes to the user's account.
 		Annotations: &mcp.ToolAnnotations{
-			ReadOnlyHint:  true,
-			OpenWorldHint: boolPtr(true),
+			ReadOnlyHint:    true,
+			DestructiveHint: boolPtr(false),
+			OpenWorldHint:   boolPtr(true),
 		},
 	}, deps.searchRecipes)
 
@@ -357,8 +358,9 @@ func registerTools(server *mcp.Server, deps *Deps) {
 		),
 		// Read-only: fetches and extracts an external URL; saves nothing.
 		Annotations: &mcp.ToolAnnotations{
-			ReadOnlyHint:  true,
-			OpenWorldHint: boolPtr(true),
+			ReadOnlyHint:    true,
+			DestructiveHint: boolPtr(false),
+			OpenWorldHint:   boolPtr(true),
 		},
 	}, deps.previewRecipe)
 
@@ -391,8 +393,9 @@ func registerTools(server *mcp.Server, deps *Deps) {
 		),
 		// Read-only: reads the user's own saved collection only (closed world).
 		Annotations: &mcp.ToolAnnotations{
-			ReadOnlyHint:  true,
-			OpenWorldHint: boolPtr(false),
+			ReadOnlyHint:    true,
+			DestructiveHint: boolPtr(false),
+			OpenWorldHint:   boolPtr(false),
 		},
 	}, deps.listMyRecipes)
 
@@ -407,8 +410,9 @@ func registerTools(server *mcp.Server, deps *Deps) {
 		),
 		// Read-only: reads one recipe from the user's own collection (closed world).
 		Annotations: &mcp.ToolAnnotations{
-			ReadOnlyHint:  true,
-			OpenWorldHint: boolPtr(false),
+			ReadOnlyHint:    true,
+			DestructiveHint: boolPtr(false),
+			OpenWorldHint:   boolPtr(false),
 		},
 	}, deps.getRecipe)
 }