fix(api): omit unset message fields from gateway JSON (#6156)

server/router/api/v1/gateway_marshaler_test.go

@@ -0,0 +1,91 @@
+package v1
+
+import (
+	"encoding/json"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	"google.golang.org/protobuf/types/known/timestamppb"
+
+	v1pb "github.com/usememos/memos/proto/gen/api/v1"
+)
+
+// TestGatewayMarshalerOmitsUnsetMessageFields pins the REST payload shape the
+// generated OpenAPI schema describes. grpc-gateway's stock marshaler writes
+// `"motionMedia": null` for a plain image attachment, which no schema declares
+// as nullable, and clients that validate against the spec — including strict
+// MCP clients, whose tool outputSchema is that same schema — reject the
+// response outright.
+func TestGatewayMarshalerOmitsUnsetMessageFields(t *testing.T) {
+	attachment := &v1pb.Attachment{
+		Name:       "attachments/plainimage1",
+		CreateTime: timestamppb.New(time.Unix(1700000000, 0).UTC()),
+		Filename:   "sunset.png",
+		Type:       "image/png",
+	}
+
+	payload := marshalThroughGateway(t, attachment)
+
+	require.NotContains(t, payload, "motionMedia", "an unset message field must be omitted, not emitted as null")
+	// Scalar defaults stay in the payload: the schema lists filename and type as
+	// required, so dropping unpopulated scalars would break validation instead.
+	require.Equal(t, "", payload["externalLink"])
+	require.Equal(t, "sunset.png", payload["filename"])
+	require.Equal(t, "image/png", payload["type"])
+	require.Equal(t, "0", payload["size"])
+}
+
+func TestGatewayMarshalerKeepsPopulatedMessageFields(t *testing.T) {
+	attachment := &v1pb.Attachment{
+		Name:     "attachments/livephoto1",
+		Filename: "walk.heic",
+		Type:     "image/heic",
+		MotionMedia: &v1pb.MotionMedia{
+			Family:  v1pb.MotionMediaFamily_APPLE_LIVE_PHOTO,
+			Role:    v1pb.MotionMediaRole_STILL,
+			GroupId: "group1",
+		},
+	}
+
+	payload := marshalThroughGateway(t, attachment)
+
+	motionMedia, ok := payload["motionMedia"].(map[string]any)
+	require.True(t, ok, "a populated message field must still be emitted: %v", payload["motionMedia"])
+	require.Equal(t, "APPLE_LIVE_PHOTO", motionMedia["family"])
+	require.Equal(t, "group1", motionMedia["groupId"])
+}
+
+// TestGatewayMarshalerKeepsEmptyCollections guards the other half of the
+// default-value behaviour: list fields the schema types as arrays must not
+// disappear when empty.
+func TestGatewayMarshalerKeepsEmptyCollections(t *testing.T) {
+	memo := &v1pb.Memo{
+		Name:       "memos/plainmemo1",
+		Content:    "",
+		State:      v1pb.State_NORMAL,
+		Visibility: v1pb.Visibility_PRIVATE,
+	}
+
+	payload := marshalThroughGateway(t, memo)
+
+	require.Equal(t, []any{}, payload["attachments"])
+	require.Equal(t, []any{}, payload["relations"])
+	require.Equal(t, "", payload["content"])
+	require.Equal(t, false, payload["pinned"])
+	// location is `optional` in the proto, so it is already omitted rather than
+	// null; property has no such marker and would otherwise be null here.
+	require.NotContains(t, payload, "location")
+	require.NotContains(t, payload, "property")
+}
+
+func marshalThroughGateway(t *testing.T, message any) map[string]any {
+	t.Helper()
+
+	data, err := newGatewayMarshaler().Marshal(message)
+	require.NoError(t, err)
+
+	payload := map[string]any{}
+	require.NoError(t, json.Unmarshal(data, &payload))
+	return payload
+}

server/router/api/v1/v1.go

@@ -9,6 +9,7 @@ import (
 	"github.com/labstack/echo/v5"
 	"github.com/pkg/errors"
 	"golang.org/x/sync/semaphore"
+	"google.golang.org/protobuf/encoding/protojson"
 
 	"github.com/usememos/memos/internal/markdown"
 	"github.com/usememos/memos/internal/profile"
@@ -65,6 +66,29 @@ func NewAPIV1Service(secret string, profile *profile.Profile, store *store.Store
 	}
 }
 
+// newGatewayMarshaler mirrors grpc-gateway's default JSON marshaler with one
+// change: EmitDefaultValues replaces EmitUnpopulated. Both keep proto3 scalar
+// defaults ("" / 0 / false) and empty lists in the payload — the generated
+// OpenAPI schema declares several of them required — but EmitUnpopulated also
+// writes `null` for every unset message field (e.g. Attachment.motion_media on
+// a plain image). No schema marks those fields nullable, so a client that
+// validates responses against the spec rejects them; the MCP tools serve the
+// same schema as their outputSchema, and strict MCP clients fail every call
+// that returns an attachment. EmitDefaultValues omits unset message fields
+// instead, which the schema already allows.
+func newGatewayMarshaler() *runtime.HTTPBodyMarshaler {
+	return &runtime.HTTPBodyMarshaler{
+		Marshaler: &runtime.JSONPb{
+			MarshalOptions: protojson.MarshalOptions{
+				EmitDefaultValues: true,
+			},
+			UnmarshalOptions: protojson.UnmarshalOptions{
+				DiscardUnknown: true,
+			},
+		},
+	}
+}
+
 // RegisterGateway registers the gRPC-Gateway and Connect handlers with the given Echo instance.
 func (s *APIV1Service) RegisterGateway(ctx context.Context, echoServer *echo.Echo) error {
 	// Shared authorizer: one source of truth for authentication and anonymous-access
@@ -108,6 +132,7 @@ func (s *APIV1Service) RegisterGateway(ctx context.Context, echoServer *echo.Ech
 
 	// Create gRPC-Gateway mux with auth middleware.
 	gwMux := runtime.NewServeMux(
+		runtime.WithMarshalerOption(runtime.MIMEWildcard, newGatewayMarshaler()),
 		runtime.WithMiddlewares(gatewayAuthMiddleware),
 	)
 	if err := v1pb.RegisterInstanceServiceHandlerServer(ctx, gwMux, s); err != nil {

server/router/mcp/README.md

@@ -198,6 +198,15 @@ not replace API authorization.
 This is deliberate: it fixes [#6022](https://github.com/usememos/memos/issues/6022),
 where collection tools returned a bare array that strict MCP clients reject.
 
+Inside that envelope the API's JSON is passed through verbatim, so the gateway's
+own encoding is part of the tool contract: whatever it emits is validated against
+the output schema resolved from the same OpenAPI spec. grpc-gateway's stock
+marshaler emits `null` for unset message fields, which no schema declares as
+nullable — `RegisterGateway` therefore installs a marshaler that omits them
+(`newGatewayMarshaler` in `server/router/api/v1/v1.go`). That fixes
+[#6139](https://github.com/usememos/memos/issues/6139), where `"motionMedia": null`
+failed every tool call returning an attachment.
+
 ## Error handling
 
 Failures are returned as MCP tool errors (`CallToolResult` with `IsError: true`

server/test/startup_test.go

@@ -415,6 +415,48 @@ func TestStartupPrivateInstanceGatewayPolicy(t *testing.T) {
 		"anonymous RSS should be unavailable on a private instance")
 }
 
+// TestStartupGatewayOmitsNullMessageFields checks the JSON the gateway actually
+// puts on the wire against the contract the generated OpenAPI schema publishes.
+// grpc-gateway's stock marshaler emits `"motionMedia": null` for a plain image
+// attachment; the schema types that field as an object, so clients validating
+// responses against it — MCP clients serve that schema as the tool outputSchema
+// — reject every memo carrying an attachment. The marshaler override lives in
+// RegisterGateway, so only a booted server proves it is wired in.
+func TestStartupGatewayOmitsNullMessageFields(t *testing.T) {
+	ctx := context.Background()
+	inst := bootInstance(ctx, t, instanceOptions{instanceURL: "http://localhost"})
+
+	inst.createAdmin(t)
+	token := inst.signIn(t)
+
+	// A 1x1 GIF: small enough to inline, real enough for the type sniffing the
+	// attachment service runs on upload.
+	const onePixelGIF = "R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7"
+	status, body := inst.do(t, http.MethodPost, "/api/v1/attachments", token, map[string]any{
+		"filename": "pixel.gif",
+		"type":     "image/gif",
+		"content":  onePixelGIF,
+	})
+	require.Equal(t, http.StatusOK, status, "creating an attachment should succeed: %s", body)
+
+	var created struct {
+		Name string `json:"name"`
+	}
+	require.NoError(t, json.Unmarshal(body, &created))
+	require.NotEmpty(t, created.Name, "the created attachment should carry a name: %s", body)
+
+	status, body = inst.do(t, http.MethodGet, "/api/v1/"+created.Name, token, nil)
+	require.Equal(t, http.StatusOK, status, "reading the attachment should succeed: %s", body)
+
+	fetched := map[string]any{}
+	require.NoError(t, json.Unmarshal(body, &fetched))
+	require.NotContains(t, fetched, "motionMedia", "an unset message field must be omitted, not null: %s", body)
+	// Scalar defaults still ship, so schema-required fields stay present.
+	require.Equal(t, "", fetched["externalLink"])
+	require.Equal(t, "pixel.gif", fetched["filename"])
+	require.Equal(t, "image/gif", fetched["type"])
+}
+
 // TestStartupDemoMode verifies demo mode boots, which exercises the seed path
 // in store.Migrate that prod-mode startups never touch.
 func TestStartupDemoMode(t *testing.T) {