diff --git a/README.md b/README.md index 8ba842a4..201df8ae 100644 --- a/README.md +++ b/README.md @@ -898,6 +898,7 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description - `sha`: Commit SHA, branch name, or tag name (string, required) - **get_file_contents** - Get file or directory contents + - `include_sha`: Whether to return file metadata (including SHA, size, type) instead of raw content (boolean, optional) - `owner`: Repository owner (username or organization) (string, required) - `path`: Path to file/directory (directories must end with a slash '/') (string, required) - `ref`: Accepts optional git refs such as `refs/tags/{tag}`, `refs/heads/{branch}` or `refs/pull/{pr_number}/head` (string, optional) diff --git a/pkg/github/__toolsnaps__/get_file_contents.snap b/pkg/github/__toolsnaps__/get_file_contents.snap index e550e8db..fedc62f3 100644 --- a/pkg/github/__toolsnaps__/get_file_contents.snap +++ b/pkg/github/__toolsnaps__/get_file_contents.snap @@ -3,7 +3,7 @@ "title": "Get file or directory contents", "readOnlyHint": true }, - "description": "Get the contents of a file or directory from a GitHub repository", + "description": "Get the contents of a file or directory from a GitHub repository. Set `include_sha` to `true` to return file metadata (including SHA, size, type) instead of raw content.", "inputSchema": { "properties": { "owner": { @@ -25,6 +25,10 @@ "sha": { "description": "Accepts optional commit SHA. If specified, it will be used instead of ref", "type": "string" + }, + "include_sha": { + "description": "Whether to return file metadata (including SHA, size, type) instead of raw content", + "type": "boolean" } }, "required": [ diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index cf71a583..6b0589a1 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -449,7 +449,7 @@ func CreateRepository(getClient GetClientFn, t translations.TranslationHelperFun // GetFileContents creates a tool to get the contents of a file or directory from a GitHub repository. func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("get_file_contents", - mcp.WithDescription(t("TOOL_GET_FILE_CONTENTS_DESCRIPTION", "Get the contents of a file or directory from a GitHub repository")), + mcp.WithDescription(t("TOOL_GET_FILE_CONTENTS_DESCRIPTION", "Get the contents of a file or directory from a GitHub repository. Set `include_sha` to `true` to return file metadata (including SHA, size, type) instead of raw content.")), mcp.WithToolAnnotation(mcp.ToolAnnotation{ Title: t("TOOL_GET_FILE_CONTENTS_USER_TITLE", "Get file or directory contents"), ReadOnlyHint: ToBoolPtr(true), @@ -472,6 +472,9 @@ func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t t mcp.WithString("sha", mcp.Description("Accepts optional commit SHA. If specified, it will be used instead of ref"), ), + mcp.WithBoolean("include_sha", + mcp.Description("Whether to return file metadata (including SHA, size, type) instead of raw content"), + ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { owner, err := RequiredParam[string](request, "owner") @@ -494,6 +497,10 @@ func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t t if err != nil { return mcp.NewToolResultError(err.Error()), nil } + includeSha, err := OptionalParam[bool](request, "include_sha") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } rawOpts := &raw.ContentOpts{} @@ -522,7 +529,8 @@ func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t t rawOpts.Ref = ref // If the path is (most likely) not to be a directory, we will first try to get the raw content from the GitHub raw content API. - if path != "" && !strings.HasSuffix(path, "/") { + // Skip raw content if include_sha is true + if !includeSha && path != "" && !strings.HasSuffix(path, "/") { rawClient, err := getRawClient(ctx) if err != nil { @@ -588,28 +596,44 @@ func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t t if sha != "" { ref = sha } - if strings.HasSuffix(path, "/") { - opts := &github.RepositoryContentGetOptions{Ref: ref} - _, dirContent, resp, err := client.Repositories.GetContents(ctx, owner, repo, path, opts) + // Try to get file/directory contents using GitHub API + opts := &github.RepositoryContentGetOptions{Ref: ref} + fileContent, dirContent, resp, err := client.Repositories.GetContents(ctx, owner, repo, path, opts) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to get file contents", + resp, + err, + ), nil + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != 200 { + body, err := io.ReadAll(resp.Body) if err != nil { - return mcp.NewToolResultError("failed to get file contents"), nil + return mcp.NewToolResultError("failed to read response body"), nil } - defer func() { _ = resp.Body.Close() }() + return mcp.NewToolResultError(fmt.Sprintf("failed to get file contents: %s", string(body))), nil + } - if resp.StatusCode != 200 { - body, err := io.ReadAll(resp.Body) - if err != nil { - return mcp.NewToolResultError("failed to read response body"), nil - } - return mcp.NewToolResultError(fmt.Sprintf("failed to get file contents: %s", string(body))), nil + // Handle directory contents + if dirContent != nil { + r, err := json.Marshal(dirContent) + if err != nil { + return mcp.NewToolResultError("failed to marshal directory contents"), nil } + return mcp.NewToolResultText(string(r)), nil + } - r, err := json.Marshal(dirContent) + // Handle file contents + if fileContent != nil { + r, err := json.Marshal(fileContent) if err != nil { - return mcp.NewToolResultError("failed to marshal response"), nil + return mcp.NewToolResultError("failed to marshal file contents"), nil } return mcp.NewToolResultText(string(r)), nil } + return mcp.NewToolResultError("Failed to get file contents. The path does not point to a file or directory, or the file does not exist in the repository."), nil } } diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index b621cec4..74dd610b 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -33,11 +33,25 @@ func Test_GetFileContents(t *testing.T) { assert.Contains(t, tool.InputSchema.Properties, "path") assert.Contains(t, tool.InputSchema.Properties, "ref") assert.Contains(t, tool.InputSchema.Properties, "sha") + assert.Contains(t, tool.InputSchema.Properties, "include_sha") assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "path"}) // Mock response for raw content mockRawContent := []byte("# Test Repository\n\nThis is a test repository.") + // Setup mock file content for include_sha test + mockFileContent := &github.RepositoryContent{ + Type: github.Ptr("file"), + Name: github.Ptr("README.md"), + Path: github.Ptr("README.md"), + SHA: github.Ptr("abc123"), + Size: github.Ptr(42), + HTMLURL: github.Ptr("https://github.com/owner/repo/blob/main/README.md"), + DownloadURL: github.Ptr("https://raw.githubusercontent.com/owner/repo/main/README.md"), + Content: github.Ptr(base64.StdEncoding.EncodeToString(mockRawContent)), + Encoding: github.Ptr("base64"), + } + // Setup mock directory content for success case mockDirContent := []*github.RepositoryContent{ { @@ -140,6 +154,68 @@ func Test_GetFileContents(t *testing.T) { expectError: false, expectedResult: mockDirContent, }, + { + name: "successful file metadata fetch with include_sha", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposContentsByOwnerByRepoByPath, + expectQueryParams(t, map[string]string{"ref": "refs/heads/main"}).andThen( + mockResponse(t, http.StatusOK, mockFileContent), + ), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "path": "README.md", + "ref": "refs/heads/main", + "include_sha": true, + }, + expectError: false, + expectedResult: mockFileContent, + }, + { + name: "successful text content fetch with include_sha false", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + raw.GetRawReposContentsByOwnerByRepoByBranchByPath, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "text/markdown") + _, _ = w.Write(mockRawContent) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "path": "README.md", + "ref": "refs/heads/main", + "include_sha": false, + }, + expectError: false, + expectedResult: mcp.TextResourceContents{ + URI: "repo://owner/repo/refs/heads/main/contents/README.md", + Text: "# Test Repository\n\nThis is a test repository.", + MIMEType: "text/markdown", + }, + }, + { + name: "successful directory metadata fetch with include_sha", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposContentsByOwnerByRepoByPath, + mockResponse(t, http.StatusOK, mockDirContent), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "path": "src/", + "include_sha": true, + }, + expectError: false, + expectedResult: mockDirContent, + }, { name: "content fetch fails", mockedClient: mock.NewMockedHTTPClient( @@ -165,7 +241,8 @@ func Test_GetFileContents(t *testing.T) { "ref": "refs/heads/main", }, expectError: false, - expectedResult: mcp.NewToolResultError("Failed to get file contents. The path does not point to a file or directory, or the file does not exist in the repository."), + expectedResult: nil, + expectedErrMsg: "failed to get file contents", }, } @@ -190,6 +267,17 @@ func Test_GetFileContents(t *testing.T) { } require.NoError(t, err) + + // Check for tool errors (API errors that return as tool results) + if tc.expectedErrMsg != "" { + require.True(t, result.IsError) + errorContent := getErrorResult(t, result) + assert.Contains(t, errorContent.Text, tc.expectedErrMsg) + return + } + + // Process successful results + require.False(t, result.IsError) // Use the correct result helper based on the expected type switch expected := tc.expectedResult.(type) { case mcp.TextResourceContents: @@ -210,9 +298,19 @@ func Test_GetFileContents(t *testing.T) { assert.Equal(t, *expected[i].Path, *content.Path) assert.Equal(t, *expected[i].Type, *content.Type) } - case mcp.TextContent: - textContent := getErrorResult(t, result) - require.Equal(t, textContent, expected) + case *github.RepositoryContent: + // File metadata fetch returns a text result (JSON object) + textContent := getTextResult(t, result) + var returnedContent github.RepositoryContent + err = json.Unmarshal([]byte(textContent.Text), &returnedContent) + require.NoError(t, err) + assert.Equal(t, *expected.Name, *returnedContent.Name) + assert.Equal(t, *expected.Path, *returnedContent.Path) + assert.Equal(t, *expected.SHA, *returnedContent.SHA) + assert.Equal(t, *expected.Size, *returnedContent.Size) + if expected.Content != nil { + assert.Equal(t, *expected.Content, *returnedContent.Content) + } } }) }