Improvements & refactoring of get_file_contents

README.md

@@ -1092,7 +1092,7 @@ Possible options:
 
 - **get_file_contents** - Get file or directory contents
   - `owner`: Repository owner (username or organization) (string, required)
-  - `path`: Path to file/directory (directories must end with a slash '/') (string, optional)
+  - `path`: Path to file/directory (string, optional)
   - `ref`: Accepts optional git refs such as `refs/tags/{tag}`, `refs/heads/{branch}` or `refs/pull/{pr_number}/head` (string, optional)
   - `repo`: Repository name (string, required)
   - `sha`: Accepts optional commit SHA. If specified, it will be used instead of ref (string, optional)

pkg/github/__toolsnaps__/get_file_contents.snap

@@ -17,7 +17,7 @@
       },
       "path": {
         "type": "string",
-        "description": "Path to file/directory (directories must end with a slash '/')",
+        "description": "Path to file/directory",
         "default": "/"
       },
       "ref": {

pkg/github/repositories.go

@@ -560,7 +560,7 @@ func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t t
 				},
 				"path": {
 					Type:        "string",
-					Description: "Path to file/directory (directories must end with a slash '/')",
+					Description: "Path to file/directory",
 					Default:     json.RawMessage(`"/"`),
 				},
 				"ref": {
@@ -608,28 +608,52 @@ func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t t
 			return utils.NewToolResultError(fmt.Sprintf("failed to resolve git reference: %s", err)), nil, nil
 		}
 
-		// 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 rawOpts.SHA != "" {
+			ref = rawOpts.SHA
+		}
 
 		var rawAPIResponseCode int
-		if path != "" && !strings.HasSuffix(path, "/") {
-			// First, get file info from Contents API to retrieve SHA
-			var fileSHA string
-			opts := &github.RepositoryContentGetOptions{Ref: ref}
-			fileContent, _, respContents, err := client.Repositories.GetContents(ctx, owner, repo, path, opts)
-			if respContents != nil {
-				defer func() { _ = respContents.Body.Close() }()
-			}
+		var fileSHA string
+		opts := &github.RepositoryContentGetOptions{Ref: ref}
+
+		// Always call GitHub Contents API first to get metadata including SHA and determine if it's a file or directory
+		fileContent, dirContent, respContents, err := client.Repositories.GetContents(ctx, owner, repo, path, opts)
+		if respContents != nil {
+			defer func() { _ = respContents.Body.Close() }()
+		}
+
+		// The path does not point to a file or directory.
+		// Instead let's try to find it in the Git Tree by matching the end of the path.
+		if err != nil || (fileContent == nil && dirContent == nil) {
+			// Step 1: Get Git Tree recursively
+			tree, response, err := client.Git.GetTree(ctx, owner, repo, ref, true)
 			if err != nil {
 				return ghErrors.NewGitHubAPIErrorResponse(ctx,
-					"failed to get file SHA",
-					respContents,
+					"failed to get git tree",
+					response,
 					err,
 				), nil, nil
 			}
-			if fileContent == nil || fileContent.SHA == nil {
-				return utils.NewToolResultError("file content SHA is nil, if a directory was requested, path parameters should end with a trailing slash '/'"), nil, nil
+			defer func() { _ = response.Body.Close() }()
+
+			// Step 2: Filter tree for matching paths
+			const maxMatchingFiles = 3
+			matchingFiles := filterPaths(tree.Entries, path, maxMatchingFiles)
+			if len(matchingFiles) > 0 {
+				matchingFilesJSON, err := json.Marshal(matchingFiles)
+				if err != nil {
+					return utils.NewToolResultError(fmt.Sprintf("failed to marshal matching files: %s", err)), nil, nil
+				}
+				resolvedRefs, err := json.Marshal(rawOpts)
+				if err != nil {
+					return utils.NewToolResultError(fmt.Sprintf("failed to marshal resolved refs: %s", err)), nil, nil
+				}
+				return utils.NewToolResultError(fmt.Sprintf("Resolved potential matches in the repository tree (resolved refs: %s, matching files: %s), but the raw content API returned an unexpected status code %d.", string(resolvedRefs), string(matchingFilesJSON), rawAPIResponseCode)), nil, nil
 			}
+			return utils.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, nil
+		}
+
+		if fileContent != nil && fileContent.SHA != nil {
 			fileSHA = *fileContent.SHA
 
 			rawClient, err := getRawClient(ctx)
@@ -702,55 +726,16 @@ func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t t
 				}
 				return utils.NewToolResultResource("successfully downloaded binary file", result), nil, nil
 			}
-			rawAPIResponseCode = resp.StatusCode
-		}
-
-		if rawOpts.SHA != "" {
-			ref = rawOpts.SHA
-		}
-		if strings.HasSuffix(path, "/") {
-			opts := &github.RepositoryContentGetOptions{Ref: ref}
-			_, dirContent, resp, err := client.Repositories.GetContents(ctx, owner, repo, path, opts)
-			if err == nil && resp.StatusCode == http.StatusOK {
-				defer func() { _ = resp.Body.Close() }()
-				r, err := json.Marshal(dirContent)
-				if err != nil {
-					return utils.NewToolResultError("failed to marshal response"), nil, nil
-				}
-				return utils.NewToolResultText(string(r)), nil, nil
-			}
-		}
-
-		// The path does not point to a file or directory.
-		// Instead let's try to find it in the Git Tree by matching the end of the path.
-
-		// Step 1: Get Git Tree recursively
-		tree, resp, err := client.Git.GetTree(ctx, owner, repo, ref, true)
-		if err != nil {
-			return ghErrors.NewGitHubAPIErrorResponse(ctx,
-				"failed to get git tree",
-				resp,
-				err,
-			), nil, nil
-		}
-		defer func() { _ = resp.Body.Close() }()
-
-		// Step 2: Filter tree for matching paths
-		const maxMatchingFiles = 3
-		matchingFiles := filterPaths(tree.Entries, path, maxMatchingFiles)
-		if len(matchingFiles) > 0 {
-			matchingFilesJSON, err := json.Marshal(matchingFiles)
-			if err != nil {
-				return utils.NewToolResultError(fmt.Sprintf("failed to marshal matching files: %s", err)), nil, nil
-			}
-			resolvedRefs, err := json.Marshal(rawOpts)
+		} else if dirContent != nil {
+			// file content or file SHA is nil which means it's a directory
+			r, err := json.Marshal(dirContent)
 			if err != nil {
-				return utils.NewToolResultError(fmt.Sprintf("failed to marshal resolved refs: %s", err)), nil, nil
+				return utils.NewToolResultError("failed to marshal response"), nil, nil
 			}
-			return utils.NewToolResultError(fmt.Sprintf("Resolved potential matches in the repository tree (resolved refs: %s, matching files: %s), but the raw content API returned an unexpected status code %d.", string(resolvedRefs), string(matchingFilesJSON), rawAPIResponseCode)), nil, nil
+			return utils.NewToolResultText(string(r)), nil, nil
 		}
 
-		return utils.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, nil
+		return utils.NewToolResultError("failed to get file contents"), nil, nil
 	})
 
 	return tool, handler