feat(tool/cloud-storage-read-object): return UTF-8 text, reject binary

MCP tool results only carry text today, so the previous base64-encoded
content was unusable by the LLM. Validate object bytes with utf8.Valid
and return plain-text content; non-UTF-8 objects surface as an
agent-fixable ErrBinaryContent error. TODO notes mark the spots to
revisit once MCP supports embedded resources.

Author: huangjiahua

docs/en/integrations/cloud-storage/tools/cloud-storage-read-object.md

@@ -3,23 +3,27 @@ title: "cloud-storage-read-object"
 type: docs
 weight: 2
 description: >
-  A "cloud-storage-read-object" tool reads the content of a Cloud Storage object and returns it as a base64-encoded string, optionally constrained to a byte range.
+  A "cloud-storage-read-object" tool reads the UTF-8 text content of a Cloud Storage object, optionally constrained to a byte range.
 ---
 
 ## About
 
 A `cloud-storage-read-object` tool fetches the bytes of a single
-[Cloud Storage object][gcs-objects] and returns them base64-encoded so that
-arbitrary binary content can be round-tripped through JSON safely.
+[Cloud Storage object][gcs-objects] and returns them as plain UTF-8 text.
+
+Only text objects are supported today: if the object bytes (or the requested
+range) are not valid UTF-8 the tool returns an agent-fixable error. This is
+because the MCP tool-result channel currently only carries text; binary
+payloads will be supported once MCP can carry embedded resources.
 
 Reads are capped at **8 MiB** per call to protect the server's memory and keep
 LLM contexts manageable; objects or ranges larger than that are rejected with
 an agent-fixable error. Use the optional `range` parameter to read a slice of
 a larger object.
 
-This tool is intended for small-to-medium textual or binary content an LLM can
-process directly. For bulk downloads of large files to the local filesystem,
-use `cloud-storage-download-object` (coming in a follow-up release).
+This tool is intended for small-to-medium textual content an LLM can process
+directly. For bulk downloads of large files to the local filesystem, use
+`cloud-storage-download-object` (coming in a follow-up release).
 
 [gcs-objects]: https://cloud.google.com/storage/docs/objects
 

internal/sources/cloudstorage/cloudstorage.go

@@ -16,9 +16,9 @@ package cloudstorage
 
 import (
 	"context"
-	"encoding/base64"
 	"fmt"
 	"io"
+	"unicode/utf8"
 
 	"cloud.google.com/go/storage"
 	"github.com/goccy/go-yaml"
@@ -136,14 +136,19 @@ func (s *Source) ListObjects(ctx context.Context, bucket, prefix, delimiter stri
 	}, nil
 }
 
-// ReadObject fetches an object's bytes and returns a map with the
-// base64-encoded content, its content type, and the number of bytes read.
-// offset and length follow storage.ObjectHandle.NewRangeReader semantics:
-// length == -1 means "read to end of object"; a negative offset means "suffix
-// from end" (in which case length must be -1). Reads larger than
-// defaultMaxReadBytes are rejected with
-// cloudstoragecommon.ErrReadSizeLimitExceeded so the caller can narrow the
-// range.
+// ReadObject fetches an object's bytes and returns a map with the UTF-8
+// content, its content type, and the number of bytes read. offset and length
+// follow storage.ObjectHandle.NewRangeReader semantics: length == -1 means
+// "read to end of object"; a negative offset means "suffix from end" (in
+// which case length must be -1). Reads larger than defaultMaxReadBytes are
+// rejected with cloudstoragecommon.ErrReadSizeLimitExceeded so the caller can
+// narrow the range. Objects whose bytes are not valid UTF-8 are rejected
+// with cloudstoragecommon.ErrBinaryContent.
+//
+// TODO: MCP tool results only carry text today, so we gate this tool on
+// utf8.Valid. When the toolbox supports non-text MCP content (embedded
+// resources, images, blobs), expand this to detect content type and return
+// binary payloads natively.
 func (s *Source) ReadObject(ctx context.Context, bucket, object string, offset, length int64) (map[string]any, error) {
 	reader, err := s.Client.Bucket(bucket).Object(object).NewRangeReader(ctx, offset, length)
 	if err != nil {
@@ -162,8 +167,13 @@ func (s *Source) ReadObject(ctx context.Context, bucket, object string, offset,
 		return nil, fmt.Errorf("failed to read object %q in bucket %q: %w", object, bucket, err)
 	}
 
+	if !utf8.Valid(data) {
+		return nil, fmt.Errorf("object %q in bucket %q: %w", object, bucket,
+			cloudstoragecommon.ErrBinaryContent)
+	}
+
 	return map[string]any{
-		"content":     base64.StdEncoding.EncodeToString(data),
+		"content":     string(data),
 		"contentType": reader.Attrs.ContentType,
 		"size":        len(data),
 	}, nil

internal/tools/cloudstorage/cloudstoragecommon/errors.go

@@ -32,6 +32,15 @@ import (
 // parameter.
 var ErrReadSizeLimitExceeded = errors.New("cloud storage read size limit exceeded")
 
+// ErrBinaryContent is returned by the source when an object's bytes are not
+// valid UTF-8. The MCP tool result channel only carries text today, so binary
+// payloads cannot be faithfully round-tripped; ProcessGCSError maps this to an
+// Agent error so the LLM knows to stop asking for this object.
+//
+// TODO: when the toolbox supports non-text MCP content (embedded resources,
+// images, blobs), remove this guard and return binary payloads directly.
+var ErrBinaryContent = errors.New("cloud storage object is not valid UTF-8 text")
+
 // ProcessGCSError classifies an error from the Cloud Storage Go client into
 // either an Agent Error (the LLM can self-correct by changing its input — bad
 // request, missing bucket/object, unsatisfiable range) or a Server Error
@@ -59,6 +68,14 @@ func ProcessGCSError(err error) util.ToolboxError {
 			err)
 	}
 
+	// Non-UTF-8 object — MCP only carries text today, so the agent should
+	// stop trying to read this object.
+	if errors.Is(err, ErrBinaryContent) {
+		return util.NewAgentError(
+			"object contains non-text (binary) bytes and cannot be returned; only UTF-8 text objects are supported",
+			err)
+	}
+
 	// GCS sentinel errors — "not found" flavours are agent-fixable.
 	if errors.Is(err, storage.ErrBucketNotExist) {
 		return util.NewAgentError("cloud storage bucket does not exist", err)

internal/tools/cloudstorage/cloudstoragecommon/errors_test.go

@@ -49,6 +49,7 @@ func TestProcessGCSError_Categorization(t *testing.T) {
 		{desc: "object not exist sentinel", in: storage.ErrObjectNotExist, category: util.CategoryAgent},
 		{desc: "wrapped object not exist", in: fmt.Errorf("wrapped: %w", storage.ErrObjectNotExist), category: util.CategoryAgent},
 		{desc: "read size limit exceeded", in: fmt.Errorf("big: %w", cloudstoragecommon.ErrReadSizeLimitExceeded), category: util.CategoryAgent},
+		{desc: "binary (non-UTF-8) content", in: fmt.Errorf("obj: %w", cloudstoragecommon.ErrBinaryContent), category: util.CategoryAgent},
 		{desc: "400 bad request", in: gcpErr(http.StatusBadRequest), category: util.CategoryAgent},
 		{desc: "401 unauthorized", in: gcpErr(http.StatusUnauthorized), category: util.CategoryServer, code: http.StatusUnauthorized},
 		{desc: "403 forbidden", in: gcpErr(http.StatusForbidden), category: util.CategoryServer, code: http.StatusForbidden},

tests/cloudstorage/cloud_storage_integration_test.go

@@ -17,7 +17,6 @@ package cloudstorage
 import (
 	"bytes"
 	"context"
-	"encoding/base64"
 	"encoding/json"
 	"fmt"
 	"io"
@@ -42,11 +41,12 @@ var (
 )
 
 const (
-	helloObject = "seed/hello.txt"
-	jsonObject  = "seed/nested/data.json"
-	largeObject = "seed/large.bin"
-	helloBody   = "hello world"
-	jsonBody    = `{"foo":"bar"}`
+	helloObject  = "seed/hello.txt"
+	jsonObject   = "seed/nested/data.json"
+	largeObject  = "seed/large.bin"
+	binaryObject = "seed/binary.bin"
+	helloBody    = "hello world"
+	jsonBody     = `{"foo":"bar"}`
 	// largeObjectSize is > the 8 MiB read cap so we can assert the size-limit
 	// agent-error path on the read_object tool.
 	largeObjectSize = (8 << 20) + 1024
@@ -158,6 +158,19 @@ func setupCloudStorageTestData(t *testing.T, ctx context.Context, client *storag
 		t.Fatalf("failed to close writer for seed object %q: %v", largeObject, err)
 	}
 
+	// Seed a small binary (non-UTF-8) object to exercise the
+	// ErrBinaryContent path on read_object.
+	binary := []byte{0xff, 0xfe, 0xfd, 0xfc}
+	bw := bkt.Object(binaryObject).NewWriter(ctx)
+	bw.ContentType = "application/octet-stream"
+	if _, err := bw.Write(binary); err != nil {
+		_ = bw.Close()
+		t.Fatalf("failed to write seed object %q: %v", binaryObject, err)
+	}
+	if err := bw.Close(); err != nil {
+		t.Fatalf("failed to close writer for seed object %q: %v", binaryObject, err)
+	}
+
 	return func(t *testing.T) {
 		cleanupCtx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
 		defer cancel()
@@ -330,9 +343,8 @@ func runCloudStorageReadObjectTest(t *testing.T, bucket string) {
 		if status != http.StatusOK {
 			t.Fatalf("unexpected status %d: %s", status, result)
 		}
-		decoded := decodeBase64Field(t, result, "content")
-		if decoded != helloBody {
-			t.Errorf("expected %q, got %q (raw %s)", helloBody, decoded, result)
+		if content := extractStringField(t, result, "content"); content != helloBody {
+			t.Errorf("expected %q, got %q (raw %s)", helloBody, content, result)
 		}
 		if ct := extractStringField(t, result, "contentType"); ct != "text/plain" {
 			t.Errorf("expected contentType text/plain, got %q", ct)
@@ -345,8 +357,8 @@ func runCloudStorageReadObjectTest(t *testing.T, bucket string) {
 		if status != http.StatusOK {
 			t.Fatalf("unexpected status %d: %s", status, result)
 		}
-		if decoded := decodeBase64Field(t, result, "content"); decoded != "hello" {
-			t.Errorf("expected %q, got %q (raw %s)", "hello", decoded, result)
+		if content := extractStringField(t, result, "content"); content != "hello" {
+			t.Errorf("expected %q, got %q (raw %s)", "hello", content, result)
 		}
 	})
 
@@ -356,8 +368,8 @@ func runCloudStorageReadObjectTest(t *testing.T, bucket string) {
 		if status != http.StatusOK {
 			t.Fatalf("unexpected status %d: %s", status, result)
 		}
-		if decoded := decodeBase64Field(t, result, "content"); decoded != "world" {
-			t.Errorf("expected %q, got %q (raw %s)", "world", decoded, result)
+		if content := extractStringField(t, result, "content"); content != "world" {
+			t.Errorf("expected %q, got %q (raw %s)", "world", content, result)
 		}
 	})
 
@@ -367,8 +379,8 @@ func runCloudStorageReadObjectTest(t *testing.T, bucket string) {
 		if status != http.StatusOK {
 			t.Fatalf("unexpected status %d: %s", status, result)
 		}
-		if decoded := decodeBase64Field(t, result, "content"); decoded != "world" {
-			t.Errorf("expected %q, got %q (raw %s)", "world", decoded, result)
+		if content := extractStringField(t, result, "content"); content != "world" {
+			t.Errorf("expected %q, got %q (raw %s)", "world", content, result)
 		}
 	})
 
@@ -419,8 +431,20 @@ func runCloudStorageReadObjectTest(t *testing.T, bucket string) {
 		if status != http.StatusOK {
 			t.Fatalf("unexpected status %d: %s", status, result)
 		}
-		if decoded := decodeBase64Field(t, result, "content"); decoded != "AAAAAAAAAA" {
-			t.Errorf("expected 10 'A' bytes, got %q (raw %s)", decoded, result)
+		if content := extractStringField(t, result, "content"); content != "AAAAAAAAAA" {
+			t.Errorf("expected 10 'A' bytes, got %q (raw %s)", content, result)
+		}
+	})
+
+	t.Run("binary object returns agent error", func(t *testing.T) {
+		result, status := invokeTool(t, "my-read-object",
+			fmt.Sprintf(`{"bucket": %q, "object": %q}`, bucket, binaryObject))
+		if status != http.StatusOK {
+			t.Fatalf("expected 200 agent error, got status %d: %s", status, result)
+		}
+		lower := strings.ToLower(result)
+		if !strings.Contains(lower, "binary") && !strings.Contains(lower, "non-text") && !strings.Contains(lower, "utf-8") {
+			t.Errorf("expected binary-content error message, got %s", result)
 		}
 	})
 }
@@ -436,15 +460,3 @@ func extractStringField(t *testing.T, result, field string) string {
 	v, _ := parsed[field].(string)
 	return v
 }
-
-// decodeBase64Field extracts a base64-encoded string field and returns its
-// decoded UTF-8 value, failing the test if decoding fails.
-func decodeBase64Field(t *testing.T, result, field string) string {
-	t.Helper()
-	encoded := extractStringField(t, result, field)
-	decoded, err := base64.StdEncoding.DecodeString(encoded)
-	if err != nil {
-		t.Fatalf("failed to base64-decode field %q: %s (encoded=%s)", field, err, encoded)
-	}
-	return string(decoded)
-}