From eed8c49dbd28a40d64d5a51ba4c44b757d519c70 Mon Sep 17 00:00:00 2001 From: Derrick Hammer Date: Sat, 22 Aug 2026 08:24:39 +0000 Subject: [PATCH 1/2] fix(mcp): surface structured data in text-only tool responses capabilities, upload_list/status/cancel, upload_file mint mode (and vault_put_file mint mode) emitted only stub prose in the Text channel while putting real data in StructuredContent. Plain-text MCP clients read only Text, so they received no actionable data (transport/source modes, upload handles, presigned URLs, curl commands). Route these results through toolargs.ResultJSONText so Text carries the same canonical JSON as StructuredContent, matching the existing mcp-result-envelope convention. Add tests asserting text-only clients receive the data. --- internal/mcp/capabilities.go | 5 +- internal/mcp/capabilities_test.go | 4 ++ .../mcp/core/transfer/upload_curl_test.go | 7 +++ internal/mcp/core/transfer/upload_file.go | 23 ++++++---- internal/mcp/upload/upload_ipfs_app.go | 21 +++++---- internal/mcp/upload/upload_tasks_test.go | 46 +++++++++++++++++++ internal/mcp/upload/upload_tools.go | 11 +++-- internal/mcp/vault/vault_put_file.go | 19 ++++---- 8 files changed, 106 insertions(+), 30 deletions(-) diff --git a/internal/mcp/capabilities.go b/internal/mcp/capabilities.go index cccb94f8..e14ad25f 100644 --- a/internal/mcp/capabilities.go +++ b/internal/mcp/capabilities.go @@ -145,7 +145,10 @@ func NewCapabilitiesDescriptor(coLocated, tunnelOpenAI, uploadFile, vaultPutFile InputSchema: toolargs.ToolSchemaFor[wizard.NoInput](), Handler: func(ctx context.Context, request model.ToolRequest) (model.ToolResult, error) { report := CurrentCapabilities(coLocated, tunnelOpenAI, uploadFile, vaultPutFile, downloadFile, vaultGetFile, dropWired, draftXFile, maxBytes) - return model.ToolResult{StructuredContent: report, Text: "Pinner capabilities."}, nil + // Text carries the same canonical JSON as StructuredContent so a + // text-only MCP client still sees the source/sink mode data instead + // of an unhelpful stub ("Pinner capabilities."). + return model.ToolResult{StructuredContent: report, Text: toolargs.ResultJSONText(report)}, nil }, } } diff --git a/internal/mcp/capabilities_test.go b/internal/mcp/capabilities_test.go index 8ad522b4..9333e8e7 100644 --- a/internal/mcp/capabilities_test.go +++ b/internal/mcp/capabilities_test.go @@ -150,6 +150,10 @@ func TestCapabilitiesDescriptorSerializes(t *testing.T) { require.Equal(t, []any{"local", "drop"}, m["download_sink_modes"]) require.Equal(t, true, m["download_file"]) require.Equal(t, true, m["vault_get_file"]) + // The text-only channel must carry the report JSON (not a bare stub) so a + // plain-text MCP client still learns the transport and source modes. + require.Contains(t, res.Text, `"transport":`) + require.Contains(t, res.Text, `"source_modes":`) } func TestCapabilitiesDescriptorIsDirectVisible(t *testing.T) { diff --git a/internal/mcp/core/transfer/upload_curl_test.go b/internal/mcp/core/transfer/upload_curl_test.go index d5e3e965..7d1cd0ac 100644 --- a/internal/mcp/core/transfer/upload_curl_test.go +++ b/internal/mcp/core/transfer/upload_curl_test.go @@ -186,6 +186,13 @@ func TestCurlUploadToolDescriptor(t *testing.T) { require.Contains(t, curlCmd, "curl") require.Contains(t, curlCmd, url) + // The text-only channel must carry the same actionable data (url + curl + // command) as StructuredContent, so a plain-text MCP client that renders no + // widget still receives what it needs to complete the upload. + require.Contains(t, res.Text, url) + require.Contains(t, res.Text, "curl") + require.Contains(t, res.Text, "upload_status") + // Invalid TTL is rejected. _, err = desc.Handler(context.Background(), model.ToolRequest{Arguments: map[string]any{"source": map[string]any{"mode": "mint"}, "ttl": "not-a-duration"}}) require.Error(t, err) diff --git a/internal/mcp/core/transfer/upload_file.go b/internal/mcp/core/transfer/upload_file.go index dc116594..985a2bfc 100644 --- a/internal/mcp/core/transfer/upload_file.go +++ b/internal/mcp/core/transfer/upload_file.go @@ -118,15 +118,22 @@ func NewUploadFileDescriptor(coLocated, tunnelOpenAI bool, pathFn UploadFileHand return model.ToolResult{}, merr } curlCmd := fmt.Sprintf("curl -sS -T %q", url) + sc := map[string]any{ + "url": url, + "curl_command": curlCmd, + "upload_handle_poll": "upload_status", + "ttl": ttl.String(), + "max_bytes": hp.maxBytes, + } + // Text carries the same JSON as StructuredContent so a text-only + // MCP client (which renders no widget) still sees the actual + // presigned URL and curl command, not just the prose instruction. + // The upload_handle itself is only produced by the presigned PUT's + // 202 response body, so it cannot appear here — the tool describes + // where to get it (curl's response) and which tool to poll with. return model.ToolResult{ - StructuredContent: map[string]any{ - "url": url, - "curl_command": curlCmd, - "upload_handle_poll": "upload_status", - "ttl": ttl.String(), - "max_bytes": hp.maxBytes, - }, - Text: "One-time upload endpoint minted. Run the curl command with your file, then poll upload_status with the returned upload_handle.", + StructuredContent: sc, + Text: toolargs.ResultJSONText(sc) + " Run the curl command with your file; the upload_handle comes back in that curl response, then poll upload_status with it.", }, nil default: // TransportOpenAI if in.Source.Mode != SourceURL && in.Source.Mode != SourceData { diff --git a/internal/mcp/upload/upload_ipfs_app.go b/internal/mcp/upload/upload_ipfs_app.go index d3c13058..7e38ea1d 100644 --- a/internal/mcp/upload/upload_ipfs_app.go +++ b/internal/mcp/upload/upload_ipfs_app.go @@ -85,15 +85,18 @@ func ipfsUploadSubmitDescriptor(hp *transfer.Upload) model.ToolDescriptor { if url == "" { return model.ToolResult{}, fmt.Errorf("failed to mint one-time upload endpoint") } + sc := map[string]any{ + "url": url, + "ttl": ttl.String(), + "max_bytes": hp.MaxBytes(), + "poll_tool": "ipfs_upload_status", + "response_body": "the 202 body carries an upload_handle the app passes to poll_tool", + } return model.ToolResult{ - StructuredContent: map[string]any{ - "url": url, - "ttl": ttl.String(), - "max_bytes": hp.MaxBytes(), - "poll_tool": "ipfs_upload_status", - "response_body": "the 202 body carries an upload_handle the app passes to poll_tool", - }, - Text: "One-time upload endpoint minted. PUT the file bytes and poll for the CID.", + StructuredContent: sc, + // Text carries the same JSON so a text-only client sees the + // actual presigned URL plus poll instructions, not a stub. + Text: toolargs.ResultJSONText(sc) + " PUT the file bytes and poll for the CID.", }, nil }, } @@ -122,7 +125,7 @@ func ipfsUploadStatusDescriptor(hp *transfer.Upload) model.ToolDescriptor { if err != nil { return model.ToolResult{}, err } - return model.ToolResult{StructuredContent: task, Text: "Upload status."}, nil + return model.ToolResult{StructuredContent: task, Text: toolargs.ResultJSONText(task)}, nil }, } } diff --git a/internal/mcp/upload/upload_tasks_test.go b/internal/mcp/upload/upload_tasks_test.go index b228c404..c5aefd78 100644 --- a/internal/mcp/upload/upload_tasks_test.go +++ b/internal/mcp/upload/upload_tasks_test.go @@ -158,6 +158,52 @@ func TestAsyncUploadStatusToolMissingHandle(t *testing.T) { require.ErrorContains(t, err, "handle is required") } +func TestAsyncUploadToolsTextCarriesData(t *testing.T) { + // Text-only MCP clients read only the Text channel. upload_file_async, + // upload_status, upload_cancel, and upload_list must put their actionable + // data there (not a bare stub) so such clients can use the handle. + release := make(chan struct{}) + mgr := transfer.NewUploadTaskManager(func(ctx context.Context, reader io.Reader, size int64, name string, wait bool) (any, error) { + select { + case <-ctx.Done(): + return nil, ctx.Err() + case <-release: + return map[string]any{"cid": "QmText"}, nil + } + }, 0) + descs := NewAsyncUploadTools(mgr) + byName := map[string]model.ToolDescriptor{} + for _, d := range descs { + byName[d.Name] = d + } + + // Seed a task directly so no network fetch is involved, then drive the + // status/list/cancel tools over that handle. + handle, err := mgr.Start(context.Background(), io.NopCloser(strings.NewReader("x")), 1, "x.txt", false) + require.NoError(t, err) + + // upload_status: Text carries the task data, not "Upload status." + status, err := byName["upload_status"].Handler(context.Background(), model.ToolRequest{Arguments: map[string]any{"handle": handle}}) + require.NoError(t, err) + require.NotEqual(t, "Upload status.", status.Text) + require.Contains(t, status.Text, handle) + + // upload_list: Text carries the tracked uploads, not "Uploads." + list, err := byName["upload_list"].Handler(context.Background(), model.ToolRequest{Arguments: map[string]any{}}) + require.NoError(t, err) + require.NotEqual(t, "Uploads.", list.Text) + require.Contains(t, list.Text, handle) + + // upload_cancel: Text carries the cancelled handle (task is still running, + // so it is cancellable). + cancel, err := byName["upload_cancel"].Handler(context.Background(), model.ToolRequest{Arguments: map[string]any{"handle": handle}}) + require.NoError(t, err) + require.NotEqual(t, "Upload cancelled.", cancel.Text) + require.Contains(t, cancel.Text, handle) + + close(release) +} + func TestUploadTaskManagerTTLEviction(t *testing.T) { mgr := transfer.NewUploadTaskManager(func(ctx context.Context, reader io.Reader, size int64, name string, wait bool) (any, error) { io.Copy(io.Discard, reader) diff --git a/internal/mcp/upload/upload_tools.go b/internal/mcp/upload/upload_tools.go index ac59c380..3952916f 100644 --- a/internal/mcp/upload/upload_tools.go +++ b/internal/mcp/upload/upload_tools.go @@ -68,7 +68,7 @@ func NewAsyncUploadTools(mgr *transfer.UploadTaskManager) []model.ToolDescriptor // unsafe. return model.ToolResult{}, err } - return model.ToolResult{StructuredContent: map[string]any{"handle": id}, Text: "Async upload started."}, nil + return model.ToolResult{StructuredContent: map[string]any{"handle": id}, Text: toolargs.ResultJSONText(map[string]any{"handle": id})}, nil }, }, { @@ -89,7 +89,7 @@ func NewAsyncUploadTools(mgr *transfer.UploadTaskManager) []model.ToolDescriptor if err != nil { return model.ToolResult{}, err } - return model.ToolResult{StructuredContent: task, Text: "Upload status."}, nil + return model.ToolResult{StructuredContent: task, Text: toolargs.ResultJSONText(task)}, nil }, }, { @@ -109,7 +109,7 @@ func NewAsyncUploadTools(mgr *transfer.UploadTaskManager) []model.ToolDescriptor if err := mgr.Cancel(in.Handle); err != nil { return model.ToolResult{}, err } - return model.ToolResult{StructuredContent: map[string]any{"handle": in.Handle, "cancelled": true}, Text: "Upload cancelled."}, nil + return model.ToolResult{StructuredContent: map[string]any{"handle": in.Handle, "cancelled": true}, Text: toolargs.ResultJSONText(map[string]any{"handle": in.Handle, "cancelled": true})}, nil }, }, { @@ -120,7 +120,10 @@ func NewAsyncUploadTools(mgr *transfer.UploadTaskManager) []model.ToolDescriptor InputSchema: toolargs.ToolSchemaFor[wizard.NoInput](), Handler: func(ctx context.Context, request model.ToolRequest) (model.ToolResult, error) { tasks := mgr.List() - return model.ToolResult{StructuredContent: map[string]any{"uploads": tasks}, Text: "Uploads."}, nil + // Text carries the same JSON as StructuredContent so a text-only + // client sees the tracked handles/status instead of a stub. + sc := map[string]any{"uploads": tasks} + return model.ToolResult{StructuredContent: sc, Text: toolargs.ResultJSONText(sc)}, nil }, }, } diff --git a/internal/mcp/vault/vault_put_file.go b/internal/mcp/vault/vault_put_file.go index bfd33edb..5290b641 100644 --- a/internal/mcp/vault/vault_put_file.go +++ b/internal/mcp/vault/vault_put_file.go @@ -125,15 +125,18 @@ func NewVaultPutFileDescriptor(coLocated, tunnelOpenAI bool, pathFn LocalPathVau return model.ToolResult{}, merr } curlCmd := fmt.Sprintf("curl -sS -T %q", url) + sc := map[string]any{ + "url": url, + "vault_path": in.VaultPath, + "curl_command": curlCmd, + "ttl": ttl.String(), + "max_bytes": vu.MaxBytes(), + } return model.ToolResult{ - StructuredContent: map[string]any{ - "url": url, - "vault_path": in.VaultPath, - "curl_command": curlCmd, - "ttl": ttl.String(), - "max_bytes": vu.MaxBytes(), - }, - Text: "One-time vault upload endpoint minted. Run the curl command with your file; the vault write completes synchronously and the response carries the vault result.", + StructuredContent: sc, + // Text carries the same JSON so a text-only client sees the + // actual presigned URL and curl command, not just prose. + Text: toolargs.ResultJSONText(sc) + " Run the curl command with your file; the vault write completes synchronously and the response carries the vault result.", }, nil default: // TransportOpenAI if in.Source.Mode != transfer.SourceURL && in.Source.Mode != transfer.SourceData { From 6ca8572f8632c98b88ceacc52fc73c4574c6402c Mon Sep 17 00:00:00 2001 From: Derrick Hammer Date: Sat, 22 Aug 2026 08:29:34 +0000 Subject: [PATCH 2/2] test(mcp-e2e): assert capabilities text carries report JSON The capabilities tool no longer returns the 'Pinner capabilities.' stub in the text channel; text now carries the same report JSON as the structured payload. Update the e2e assertion to match. --- tests/sunpeak/mcp-e2e/meta-tools.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/sunpeak/mcp-e2e/meta-tools.test.ts b/tests/sunpeak/mcp-e2e/meta-tools.test.ts index 489e1339..090ee65a 100644 --- a/tests/sunpeak/mcp-e2e/meta-tools.test.ts +++ b/tests/sunpeak/mcp-e2e/meta-tools.test.ts @@ -100,8 +100,10 @@ test('capabilities tool returns declared capabilities', async ({ mcp }) => { const result = await mcp.callTool('capabilities', {}); expect(result).not.toBeError(); - // text content is the human label; the capability report is structured. - expect(result).toHaveTextContent('Pinner capabilities'); + // text content carries the report JSON (same data as the structured payload) + // so a text-only client sees the transport + source modes, not a stub label. + expect(result).toHaveTextContent('transport'); + expect(result).toHaveTextContent('source_modes'); // Locked from a live probe: stdio transport advertises only `path` sourcing. expect(result).toHaveStructuredContent({ transport: 'stdio' });